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

1 stars 1 forks source link

Users can get tickets for 10% less if they set themselves as frontend operators #400

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 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L151-L157 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L249-L257 https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotteryMath.sol#L119-L130

Vulnerability details

Impact

When users buy a ticket, they can set an address for the Frontend Operator where they bought it. Then Frontend Operators can claim Frontend Rewards of 10% on the sold tickets.

But nothing prevents users from settings theirselves as the Frontend Operator, and getting back 10% of the ticket price instantly.

Proof of Concept

• User set himself as frontend when buying tickets through the buyTickets function

• User calls the claimRewards function with rewardType = LotteryRewardType.FRONTEND as the parameters

• User will receive 10% of the ticket price

First the user buys the ticket, setting himself as the frontend:

// File: src/Lottery.sol

110:    function buyTickets(
111:        uint128[] calldata drawIds,
112:        uint120[] calldata tickets,
113:        address frontend, // @audit-info Users can set theirselves as `frontend`
114:        address referrer
115:    )
116:        external
117:        override
118:        requireJackpotInitialized
119:        returns (uint256[] memory ticketIds)
120:    {
121:        if (drawIds.length != tickets.length) {
122:            revert DrawsAndTicketsLenMismatch(drawIds.length, tickets.length);
123:        }
124:        ticketIds = new uint256[](tickets.length);
125:        for (uint256 i = 0; i < drawIds.length; ++i) {
126:            ticketIds[i] = registerTicket(drawIds[i], tickets[i], frontend, referrer);
127:        }
128:        referralRegisterTickets(currentDraw, referrer, msg.sender, tickets.length);
129:        frontendDueTicketSales[frontend] += tickets.length; // @audit-info Tickets sold via a "frontend" address
130:        rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);
131:    }

Link to code

Then the user claims the 10% ticket price as a reward, calling the claimRewards function with the rewardType = LotteryRewardType.FRONTEND parameter.

The beneficiary will be the msg.sender

// File: src/Lottery.sol

151:    function claimRewards(LotteryRewardType rewardType) external override returns (uint256 claimedAmount) {
152:        address beneficiary = (rewardType == LotteryRewardType.FRONTEND) ? msg.sender : stakingRewardRecipient;
153:        claimedAmount = LotteryMath.calculateRewards(ticketPrice, dueTicketsSoldAndReset(beneficiary), rewardType);
154:
155:        emit ClaimedRewards(beneficiary, claimedAmount, rewardType);
156:        rewardToken.safeTransfer(beneficiary, claimedAmount);
157:    }

Link to code

The due tickets corresponding to the claiming address were previously set when the ticket was bought.

// File: src/Lottery.sol

249:    function dueTicketsSoldAndReset(address beneficiary) private returns (uint256 dueTickets) {
250:        if (beneficiary == stakingRewardRecipient) {
251:            dueTickets = nextTicketId - claimedStakingRewardAtTicketId;
252:            claimedStakingRewardAtTicketId = nextTicketId;
253:        } else {
254:            dueTickets = frontendDueTicketSales[beneficiary]; //@audit-info Previously set when the ticket was bought
255:            frontendDueTicketSales[beneficiary] = 0;
256:        }
257:    }

Link to code

The function that calculates the rewards doesn't perform any additional check for Frontend Rewards.

// File: src/LotteryMath.sol

16:     uint256 public constant FRONTEND_REWARD = 10 * PercentageMath.ONE_PERCENT;

119:    function calculateRewards(
120:        uint256 ticketPrice,
121:        uint256 ticketsSold,
122:        LotteryRewardType rewardType
123:    )
124:        internal
125:        pure
126:        returns (uint256 dueRewards)
127:    {
128:        uint256 rewardPercentage = (rewardType == LotteryRewardType.FRONTEND) ? FRONTEND_REWARD : STAKING_REWARD;
129:        dueRewards = (ticketsSold * ticketPrice).getPercentage(rewardPercentage);
130:    }

Link to code

Tools Used

Manual review

Recommended Mitigation Steps

Return zero rewards for Frontend Operators that don't reach a minimum of tickets sold, such as the check done in the referral system, that requires users to have a minimum of referrals in order to claim the referrer rewards.

This way, it discourages users trying to game the system by referring themselves as Frontend Operators, as they would need to buy at least a minimum amount of tickets to receive the reward.

    function calculateRewards(
        uint256 ticketPrice,
        uint256 ticketsSold,
        LotteryRewardType rewardType
    )
        internal
        pure
        returns (uint256 dueRewards)
    {
+        if (rewardType == LotteryRewardType.FRONTEND && ticketsSold < minimumEligibleFrontendReferrals) {
+          return 0;
+        }
        uint256 rewardPercentage = (rewardType == LotteryRewardType.FRONTEND) ? FRONTEND_REWARD : STAKING_REWARD;
        dueRewards = (ticketsSold * ticketPrice).getPercentage(rewardPercentage);
    }
c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #483

c4-judge commented 1 year ago

thereksfour changed the severity to QA (Quality Assurance)