search

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

1 stars 1 forks source link

An attacker can always win the jackpot, as long as there are other jackpot winners. #298

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/TicketUtils.sol#L83-L99

Vulnerability details

Impact

Detailed description of the impact of this finding.

An attacker can simply select all the possible numbers and then win. The winning tiers are calculated by counting the amount of times the 1's of two different bit sets hit. So for example 00000000000000000000000000000000001 and 00000000000000000000000000000000001 the 1's at the end would hit. "The & (bitwise AND) takes two numbers as operands and does AND on every bit of two numbers. The result of AND is 1 only if both bits are 1." So, for the values 1010 and 1001, the and result would be 1000.

The problem is that the code uses bitwise & (https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/TicketUtils.sol#L94), so if the attacker passes a number 11111111111111111111111111111111111, no matter the winning ticket combination, it's going to hit every single one of the winning tickets 1's, so then the attacker immediately becomes the jackpot winner.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

  1. Attacker calls buyTicket(), selects the current running drawId, passes the uint120 value 11111111111111111111111111111111111 as the ticket and the frontend and refferer (what these addresses are doesn't matter). https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L110-L131

  2. Attacker now owns a ticket with all the possible numbers as the combination. Now he waits for the draw to finish.

  3. Draw executes, the winning ticket is constructed.

  4. The attacker calls claimWinningTicket() and claims his ticket. He hits all the winning ticket's numbers and becomes the jackpot winner. https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L259-L269

To my understanding this is only achievable if there are other jackpot winners, simply because the attackers ticket is structurally different than the winning ticket, e.g. the winning ticket 11111111111100110111111111101111110, the attacker ticket 11111111111111111111111111111111111. These are different, so if there aren't any jackpot winners the winAmount[][] https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L209 is never initialized with any value, so it's practically useless. But if there are jackpot winners, the attacker can simply receive the winning.

Tools Used

manual review

Recommended Mitigation Steps

  1. Don't allow the user to select all possible 35 numbers while buying the ticket. Only allow to select 7 numbers.

  2. Another way to solve it would be to compare if the uint values of the winning ticket and the user submitted ticket are the same in case the user is a jackpot winner. Because the number with all 1's will be different than the winning ticket, the attacker can't claim the prize.

thereksfour commented 1 year ago

Consider winningTicket = 1000, ticket = 1111, result winTier = 1, not 4

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid