Lost of reward token

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L271

Vulnerability details

Impact

The current implementation of returnUnclaimedJackpotToThePot only works correctly and reclaims the profit by updating currentNetProfit when there is a jackpot winner. In the case where there isn't one and some winning tickets go unclaimed, the currentNetProfit is not calculated correctly resulting in permanent lost of funds since there's no alternative way of transferring the reward token away from the Lottery contract.

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "./LotteryTestBase.sol";
import "../src/Lottery.sol";
import "./TestToken.sol";
import "test/TestHelpers.sol";

contract MoreLotteryTest is LotteryTestBase {
    address public constant OWNER = address(0x111);

    address public constant USER = address(123);

    function testNonJackpotWinClaimable() public {
        uint128 drawId = lottery.currentDraw();
        uint256 ticketId = initTickets(drawId, 0x8E);

        // this will give winning ticket of 0x0F so 0x8E will have 3/4
        finalizeDraw(0);

        uint8 winTier = 3;
        checkTicketWinTier(drawId, 0x8E, winTier);
        // claimWinnings(drawId, ticketId, winTier, fixedRewards[winTier]);
        int256 beforeReturn = lottery.currentNetProfit();
        console.logInt(beforeReturn);
        for (uint256 i = 0; i < 52; ++i) {
            finalizeDraw(0);
        }
        int256 afterReturn = lottery.currentNetProfit();
        console.logInt(afterReturn);
        assertFalse(beforeReturn == afterReturn);
    }

    function testJackpotWinClaimable() public {
        uint128 drawId = lottery.currentDraw();
        uint256 ticketId = initTickets(drawId, 0x0F);

        // this will give winning ticket of 0x0F so 0x8E will have 3/4
        finalizeDraw(0);

        uint8 winTier = 4;
        checkTicketWinTier(drawId, 0x0F, winTier);
        // claimWinnings(drawId, ticketId, winTier, fixedRewards[winTier]);
        int256 beforeReturn = lottery.currentNetProfit();
        console.logInt(beforeReturn);
        for (uint256 i = 0; i < 52; ++i) {
            finalizeDraw(0);
        }
        int256 afterReturn = lottery.currentNetProfit();
        console.logInt(afterReturn);
        assertFalse(beforeReturn == afterReturn);
    }

    function initTickets(uint128 drawId, uint120 numbers) private returns (uint256 ticketId) {
        vm.startPrank(USER);
        rewardToken.mint(TICKET_PRICE);
        rewardToken.approve(address(lottery), TICKET_PRICE);
        ticketId = buyTicket(drawId, numbers, address(0));
        // buy the same tickets to increase nonJackpot count
        buySameTickets(drawId, uint120(0xF0), address(0), 10);
        vm.stopPrank();
    }

    function checkTicketWinTier(uint128 drawId, uint120 ticket, uint256 expectedWinTier) private {
        uint120 winningTicket = lottery.winningTicket(drawId);
        uint256 winTier = TicketUtils.ticketWinTier(ticket, winningTicket, SELECTION_SIZE, SELECTION_MAX);
        assertEq(winTier, expectedWinTier);
    }

}

Tools Used

forge test

Recommended Mitigation Steps

Update the implementation of returnUnclaimedJackpotToThePot to account for the case where there's no jackpot winner.

[c4-judge]

thereksfour marked the issue as duplicate of #303

[c4-judge]

thereksfour changed the severity to QA (Quality Assurance)

[c4-judge]

thereksfour marked the issue as grade-c