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

1 stars 1 forks source link

`claimPerDraw()` can accounts for referrals after the ticket registration deadline #394

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L125-L128

Vulnerability details

When accounting for referrals, the beforeTicketRegistrationDeadline ensure tickets cannot be purchased after the draw registration deadline.

The issue is that the referral registration happens for the current draw regardless of which draw the tickets are purchased for

src/Lottery.sol
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);//@audit can bypass registration deadline

This will increase the number of unclaimed tickets of the referrer for the current draw, even though the deadline has passed

src/ReferralSystem.sol
69:             unclaimedTickets[currentDraw][referrer].referrerTicketCount += uint128(numberOfTickets);

Which is the amount used when computing their claim in claimPerDraw.

Impact

Referrers are able to bypass the registration deadline, effectively stealing current a share of the draw rewards

Tools Used

Manual analysis

Recommended Mitigation Steps

The referral registration upon buyTicket should account for the draw the tickets are bought for.

src/Lottery.sol
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);
+                referralRegisterTickets(drawIds[i], referrer, msg.sender, 1);
127:         }
-128:         referralRegisterTickets(currentDraw, referrer, msg.sender, tickets.length);//@audit can bypass registration deadline
c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid

thereksfour commented 1 year ago

It may be intentional. Referrer can be rewarded for the epoch in which the referred person buys the lottery ticket. Consider QA. Open it for sponsors.

c4-judge commented 1 year ago

thereksfour marked the issue as nullified

c4-judge commented 1 year ago

thereksfour marked the issue as not nullified

c4-judge commented 1 year ago

thereksfour marked the issue as primary issue

c4-sponsor commented 1 year ago

TutaRicky marked the issue as sponsor disputed

TutaRicky commented 1 year ago

The issue is that the referral registration happens for the current draw regardless of which draw the tickets are purchased for... This is by design decision.

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid