Forgeries contest - 0xdeadbeef0x's results

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L173

Vulnerability details

Impact

Owners can leverage the insufficient funds in Chainlink subscription to successfully call startDraw and not appoint a winner.

This is medium because theoretically, users can add funds to the subscriptions if such a case is noticed as anyone can fund a subscription. It is less likely, because it would require them to pay extra funds for a chance to win the raffle.

Proof of Concept

The bug is because fulfillRandomWords will revert. This is due to Chainlink calculating that the payment is more than the balance from the subscription: https://github.com/smartcontractkit/chainlink/blob/286a65065fcfa5e1b2362745079cdc218e40e68d/contracts/src/v0.8/VRFCoordinatorV2.sol#L595

(See previous submission: Raffle is insolvent if owner cancels Chainlink subscription [194])

Foundry POC

The following POC demonstrates the creation of a raffle and a successful startDraw. No funding of the subscription. Users will not able to call hasUserWon After lock period. owner will withdraw the NFT.

Add the following test to VRFNFTRandomDraw.t.sol: https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/test/VRFNFTRandomDraw.t.sol#L541

    function test_insufficiantFunding() public {
        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: 2 weeks,
                keyHash: bytes32(
                    0x79d3d8832d904592c0bf9818b621522c988bb8b0c05cdc3b15aea1b6e8db0c15
                ),
                subscriptionId: subscriptionId
            })
        );
        vm.label(consumerAddress, "drawing instance");

        mockCoordinator.addConsumer(subscriptionId, consumerAddress);

        VRFNFTRandomDraw drawing = VRFNFTRandomDraw(consumerAddress);
        targetNFT.setApprovalForAll(consumerAddress, true);

        // Start draw
        uint256 drawingId = drawing.startDraw();

        vm.stopPrank();

        // Revert because insufficiant funds
        vm.expectRevert();
        mockCoordinator.fulfillRandomWords(drawingId, consumerAddress);

        vm.warp(2 weeks); // forward two weeks

        // validate no number was chosen (Although start draw was called)
        vm.expectRevert(IVRFNFTRandomDraw.NEEDS_TO_HAVE_CHOSEN_A_NUMBER.selector);
        drawing.hasUserWon(address(this));

        vm.prank(admin);
        drawing.lastResortTimelockOwnerClaimNFT();

        // validate admin received his NFT
        assertEq(targetNFT.balanceOf(admin), 1);
    }

To run the POC, execute: forge test -m "test_insufficiantFunding" -vvvv

Tools Used

VS Code, Foundry

Recommended Mitigation Steps

Validate in the initialization of the contract or the startDraw that the amount of funds currently in the subscription are sufficient for a minimum amount of redraws (related to the lock period). If possible, automatically transfer a portion of the revenue of the raffle tickets to pay for the subscription funding.

Here is the function that can receive the amount of funds in the subscription: https://github.com/smartcontractkit/chainlink/blob/286a65065fcfa5e1b2362745079cdc218e40e68d/contracts/src/v0.8/VRFCoordinatorV2.sol#L685

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L203

Vulnerability details

Impact

The owner can decide not to redraw. Therefore if no winner is selected the first time, the owner can wait for the lock period to end and get their NFT back.

Proof of Concept

redraw function is set with the onlyOwner modifier. It can only be called by the owner. https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L203:

    function redraw() external onlyOwner returns (uint256) {

Tools Used

VS Code

Recommended Mitigation Steps

The protocol has placed a lot of trust in the owner. Users should be able to call redraw as well.