code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/ReferralSystem.sol#L99-L100 https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/ReferralSystem.sol#L105

Vulnerability details

Impact

If there is a sufficiently small referrer reward and correspondingly large amount of eligible referred tickets sold for a given draw, there is potential for the reward for one ticket to = 0, leading to loss of rewards.

Proof of Concept

Example context for player rewards (same situation applies for referrer rewards):

Assume all tickets are bought by 1 user
playerRewardForDraw = 99
ticketsSoldDuringDraw = 100

ReferralSystem.referralDrawFinalize() is called when finalizing a lottery draw, and in this case, the user should receive the full 99 from playerRewardForDraw.

However, since in referralDrawFinalize() playerRewardsPerDrawForOneTicket[drawFinalized] = playerRewardForDraw / ticketsSoldDuringDraw rounds down to 0, and later in claimPerDraw() claimedReward += playerRewardsPerDrawForOneTicket[drawId] * _unclaimedTickets.playerTicketCount;, although the user should receive the full allocated amount of the reward (99), they receive 0.

Tools Used

Manual review

Recommended Mitigation Steps

Some suggestions:

Ensure rewards are stored according to the # decimals in the underlying token (ie: 1 LOT is stored as 1e18), so that it is very unlikely to have # tickets sold > reward
In referralDrawFinalize(), store the draw's reward components (ie: totalReward & totalTicketCount) as a struct and perform the division after the multiplication in claimPerDraw in order to minimize the precision loss. (ie: this would become userReward = userTicketCount * totalReward / totalTicketCount)