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

1 stars 1 forks source link

Lottery Insolvency can lead to unclaimable winning tickets despite paying out Frontend and Staking rewards #398

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#L175 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotterySetup.sol#L80 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotterySetup.sol#L161 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L151

Vulnerability details

Impact

Lottery Insolvency can lead to unclaimable winning tickets despite paying out Frontend and Staking rewards

Proof of Concept

When distributing the winning tokens, it is possible that there is an insufficient balance to be able to pay winning tickets while Frontend and Staking rewards are still claimable.

The call to rewardToken.safeTransfer can revert due to insufficient balance.

    function claimWinningTickets(uint256[] calldata ticketIds) external override returns (uint256 claimedAmount) {
        uint256 totalTickets = ticketIds.length;
        for (uint256 i = 0; i < totalTickets; ++i) {
            claimedAmount += claimWinningTicket(ticketIds[i]);
        }
        rewardToken.safeTransfer(msg.sender, claimedAmount);
    }

This is possible because prizes have a fixed minimum that could lead to insufficient balance to cover the winnings.

Furthermore, Frontend and Staking rewards can be claimed at any time further depleting the available balance to cover winnings.

    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);
    }

The following test demonstrates a contrived example of the lottery going into debt while still paying out rewards -

    function oneRound() private {
        uint128 currentDraw = lottery.currentDraw();

        uint256[] memory ticketIds; //= new uint256[](1);

        vm.startPrank(user);
        ticketIds = _buyTickets(currentDraw, uint120(0x0F), user, user, 1);
        vm.stopPrank();

        // this is TestToken aka DAI
        vm.prank(user);
        lottery.claimRewards(LotteryRewardType.FRONTEND);

        rewardToken.balanceOf(address(lottery));

        vm.prank(user);
        lottery.claimRewards(LotteryRewardType.STAKING);

        // this is DAI
        vm.prank(user);
        staking.getReward();

        uint balance = rewardToken.balanceOf(address(lottery));

        executeDraw();

        lotteryToken.balanceOf(user);

        uint128[] memory drawIds = new uint128[](1);
        drawIds[0] = 0;

        // this is LOT needs current draw executed 
        vm.prank(user);
        referralSystem.claimReferralReward(drawIds);

        (uint price, ) = lottery.claimable(ticketIds[0]);

        if (balance > price) {
          vm.prank(user);
          lottery.claimWinningTickets(ticketIds);
        } else {
          vm.prank(user);
          vm.expectRevert();
          lottery.claimWinningTickets(ticketIds);
        }

        rewardToken.balanceOf(user);
        rewardToken.balanceOf(address(lottery));

        lottery.currentNetProfit();
    }

    function testRekt() public {
        vm.prank(address(lottery));
        stakingToken.mint(user, 1);

        vm.startPrank(user);
          stakingToken.approve(address(staking), 1);
          staking.stake(1);
        vm.stopPrank();

        for (uint i; i < 100; ++i) {
          oneRound();
        }
    }

https://gist.github.com/alpeware/9dc133b0282cfa662cc0bb13de6dfbfc#file-referralsystembase-sol-L266

Tools Used

Foundry

Recommended Mitigation Steps

The case of insolvency should be handled specifically and dept payments accordingly.

A few ideas -

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Overinflated severity

c4-judge commented 1 year ago

thereksfour marked the issue as satisfactory

c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #316

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid