code-423n4 / 2023-03-wenwin-findings

1 stars 1 forks source link

`frontend` param is lacking validation which may results in rewards locked forever #55

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

In Lottery.sol contract any user can buy tickets for lottery. Call to buyTickets() takes 4 parameters and 1 of those is address frontend. After passing checks and bounds if tickets are minted then number of minted tickets are stored in this mapping frontendDueTicketSales[frontend]. It will be used later.

    function buyTickets(
        uint128[] calldata drawIds,
        uint120[] calldata tickets,
        address frontend,
        address referrer
    )
        external
        override
        requireJackpotInitialized
        returns (uint256[] memory ticketIds)
    {
        if (drawIds.length != tickets.length) {
            revert DrawsAndTicketsLenMismatch(drawIds.length, tickets.length);
        }
        ticketIds = new uint256[](tickets.length);
        for (uint256 i = 0; i < drawIds.length; ++i) {
            ticketIds[i] = registerTicket(drawIds[i], tickets[i], frontend, referrer);
        }
        referralRegisterTickets(currentDraw, referrer, msg.sender, tickets.length);
        frontendDueTicketSales[frontend] += tickets.length;
        rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);
    }

Now at any point in future frontend operator aka address frontend can call claimRewards to claim rewards from Lottery contract.

    function claimRewards(LotteryRewardType rewardType) external override returns (uint256 claimedAmount) {
        address beneficiary = (rewardType == LotteryRewardType.FRONTEND) ? msg.sender : stakingRewardRecipient;
        claimedAmount = LotteryMath.calculateRewards(ticketPrice, dueTicketsSoldAndReset(beneficiary), rewardType);

        emit ClaimedRewards(beneficiary, claimedAmount, rewardType);
        rewardToken.safeTransfer(beneficiary, claimedAmount);
    }

Issue

buyTickets() has no validation of != address(0) for address frontend. When 0x0 address is passed then function will record the frontendDueTicketSales for 0x0 and this frontendDueTicketSales rewards are not claimable by any address.

Impact

Rewards stored at frontendDueTicketSales[0x0] remains unclaimed and never returned to Jackpot either. Which makes rewards locked in contract forever.

Proof of Concept

For POC, I made frontendDueTicketSales variable public which makes verification easy. Below POC shows that it is possible to send 0x0 as frontend address and tickets will be stored against 0x0 which eventually results in rewards locked forever.

    function testBuyTicketFromZeroFrontEnd() public {
        uint128 currentDraw = lottery.currentDraw();
        uint256 initialBalance = rewardToken.balanceOf(address(lottery));

        assertEq(lottery.frontendDueTicketSales(address(0)), 0);
        vm.startPrank(USER);
        rewardToken.mint(5 ether);
        rewardToken.approve(address(lottery), 10 ether);

        uint128[] memory drawIds = new uint128[](1);
        drawIds[0] = currentDraw;
        uint120[] memory tickets = new uint120[](1);
        tickets[0] = uint120(0x0F);

        // Passing address(0) as frontend address 3rd param
        lottery.buyTickets(drawIds, tickets, address(0), address(0));

        assertEq(rewardToken.balanceOf(address(lottery)), initialBalance + TICKET_PRICE);

        vm.stopPrank();
        assertGt(lottery.frontendDueTicketSales(address(0)), 0);
    }

Recommended Mitigation Steps

There are two possible solution for this to avoid this situation.

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Overinflated severity