Forgeries contest - evan's results

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L75-L138 https://github.com/code-423n4/2022-12-forgeries/blob/main/src/interfaces/IVRFNFTRandomDraw.sol#L73-L81

Vulnerability details

Impact

Creators can create multiple draws with the same prize, but only the first draw to call startDraw will have the prize. This can trick users into entering raffle pools that does not have a prize.

Furthermore, the prize token can also be one of the tokens in the raffle pool. If the prize token gets drawn, then no one can get the prize since the contract is the winner.

Proof of Concept

Run the following test. Test_PrizeReuse creates 2 draws with the same prize. The second draw fails when calling startDraw. Test_DrawingPrize creates a draw where the prize is in the drawing pool. It proves that it's possible for the contract itself to win the raffle at the end of the draw.

pragma solidity 0.8.16;

import "forge-std/Test.sol";
import "forge-std/console2.sol";

import {VRFCoordinatorV2Mock} from "@chainlink/contracts/src/v0.8/mocks/VRFCoordinatorV2Mock.sol";
import {VRFCoordinatorV2} from "@chainlink/contracts/src/v0.8/VRFCoordinatorV2.sol";
import {VRFNFTRandomDraw} from "../src/VRFNFTRandomDraw.sol";
import {VRFNFTRandomDrawFactory} from "../src/VRFNFTRandomDrawFactory.sol";

import {IOwnableUpgradeable} from "../src/ownable/IOwnableUpgradeable.sol";

import {IERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/IERC721EnumerableUpgradeable.sol";

import {IVRFNFTRandomDraw} from "../src/interfaces/IVRFNFTRandomDraw.sol";
import {IVRFNFTRandomDrawFactory} from "../src/interfaces/IVRFNFTRandomDrawFactory.sol";

import {MockNFT} from "./mocks/MockNFT.sol";
import {MockERC20} from "./mocks/MockERC20.sol";

contract VRFNFTRandomDrawVuln is Test {
    MockNFT targetNFT;
    MockNFT drawingNFT;
    MockERC20 linkTokens;
    VRFNFTRandomDrawFactory factory;

    VRFCoordinatorV2Mock mockCoordinator;

    address user = address(0x2134);
    address admin = address(0x0132);

    uint64 subscriptionId;


    function setUp() public {
        vm.label(user, "USER");
        vm.label(admin, "ADMIN");

        subscriptionId = 1337;

        targetNFT = new MockNFT("target", "target");
        vm.label(address(targetNFT), "TargetNFT");
        drawingNFT = new MockNFT("drawing", "drawing");
        vm.label(address(drawingNFT), "DrawingNFT");
        linkTokens = new MockERC20("link", "link");
        vm.label(address(linkTokens), "LINK");

        mockCoordinator = new VRFCoordinatorV2Mock(0.1 ether, 1000);

        VRFNFTRandomDraw drawImpl = new VRFNFTRandomDraw(mockCoordinator);
        // Unproxied/unowned factory
        factory = new VRFNFTRandomDrawFactory(address(drawImpl));

        vm.prank(admin);
        subscriptionId = mockCoordinator.createSubscription();
    }
    

    function test_PrizeReuse() 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 consumerAddress1 = factory.makeNewDraw(
            IVRFNFTRandomDraw.Settings({
                token: address(targetNFT),
                tokenId: 0,
                drawingToken: address(drawingNFT),
                drawingTokenStartId: 0,
                drawingTokenEndId: 8,
                drawBufferTime: 1 hours,
                recoverTimelock: 2 weeks,
                keyHash: bytes32(
                    0x79d3d8832d904592c0bf9818b621522c988bb8b0c05cdc3b15aea1b6e8db0c15
                ),
                subscriptionId: subscriptionId
            })
        );
        address consumerAddress2 = factory.makeNewDraw(
            IVRFNFTRandomDraw.Settings({
                token: address(targetNFT),
                tokenId: 0,
                drawingToken: address(drawingNFT),
                drawingTokenStartId: 3,
                drawingTokenEndId: 10,
                drawBufferTime: 1 hours,
                recoverTimelock: 2 weeks,
                keyHash: bytes32(
                    0x79d3d8832d904592c0bf9818b621522c988bb8b0c05cdc3b15aea1b6e8db0c15
                ),
                subscriptionId: subscriptionId
            })
        );
        vm.label(consumerAddress1, "drawing instance 1");
        vm.label(consumerAddress2, "drawing instance 2");

        mockCoordinator.addConsumer(subscriptionId, consumerAddress1);
        mockCoordinator.addConsumer(subscriptionId, consumerAddress2);
        mockCoordinator.fundSubscription(subscriptionId, 100 ether);

        VRFNFTRandomDraw drawing1 = VRFNFTRandomDraw(consumerAddress1);
        VRFNFTRandomDraw drawing2 = VRFNFTRandomDraw(consumerAddress2);

        targetNFT.setApprovalForAll(consumerAddress1, true);
        targetNFT.setApprovalForAll(consumerAddress2, true);

        drawing1.startDraw();

        vm.expectRevert();
        drawing2.startDraw();
    }

    function test_DrawingPrize() public {

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

        address consumerAddress = factory.makeNewDraw(
            IVRFNFTRandomDraw.Settings({
                token: address(targetNFT),
                tokenId: 0,
                drawingToken: address(targetNFT),
                drawingTokenStartId: 0,
                drawingTokenEndId: 2,
                drawBufferTime: 1 hours,
                recoverTimelock: 2 weeks,
                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);

        while (!drawing.hasUserWon(consumerAddress)){
            vm.warp(block.timestamp + 3 hours);
            drawingId = drawing.redraw();
            mockCoordinator.fulfillRandomWords(drawingId, consumerAddress);
        }

        assertEq(drawing.hasUserWon(consumerAddress), true);

    }
}

Tools Used

VSCode, foundry

Recommended Mitigation Steps

Create a mapping between prize tokens and VRFNFTRandomDraw contracts in VRFNFTRandomDrawFactory.sol to prevent prize re-use. Also, add a check in VRFNFTRandomDraw.sol initialize() function to ensure that the prize is not in the raffle pool.

Lines of code

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

Vulnerability details

Impact

_settings.drawBufferTime can be set as high as 7 months, _settings.recoverTimelock can be set as late as 7 years later.

Proof of Concept

https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L31-L33 I don't think there are 30 weeks in a month.

Tools Used

VSCode

Recommended Mitigation Steps

uint256 immutable MONTH_IN_SECONDS = 3600 * 24 * 30; Or better, consider using time units