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

1 stars 1 forks source link

Ticket buyer can add himself as a frontend and gain rewards #483

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#L110 https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L151

Vulnerability details

Impact

The ticket buyer can set himself as a frontend and the ticket buyer can claim rewards for each purchased ticket.

As the documentation says the frontend can gain 10% of rewards of each sale and the ticket buyer can get that percentage money back

Proof of Concept

There are not any validation that the ticket buyer can be added himself as a frontend.

File: Lottery.sol
110:     function buyTickets(
111:         uint128[] calldata drawIds,
112:         uint120[] calldata tickets,
113:         address 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;
130:         rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);
131:     }

Tools used

Foundry/Vscode

Recommended Mitigation Steps

Check that the ticket buyer is not the same as the frontend.

thereksfour commented 1 year ago

Should be intentional, consider QA

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

This is by design correct (we don't want to have frontend whitelisting).

c4-judge commented 1 year ago

thereksfour changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

thereksfour marked the issue as grade-c