code-423n4 / 2024-02-thruster-findings

2 stars 1 forks source link

Winner can be manipulated by frontrunning #8

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-thruster/blob/main/thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol#L310

Vulnerability details

Proof of Concept

Winner of prize depends on 2 things: random number and currentTickets.

Random number is provided by pyth and currentTickets is increased with tickets that are deposited in the round.

In case if attacker can manipulate with currentTickets then he can manipulate with result.

Because of that attacker can frontrun setWinningTickets for the first prize(because when first prize is distributed, then you can't add more tickets) and add more tickets if he have them to change currentTickets. This will be profitable for him only for such randoms that will make him a winner.

Also it is possible that owner will select winner for not first prize first(in random order), but in another order. Then it is possible for attacker to add tickets still, as only when winners fro prize 0 are selected, then tickets adding is blocked for the round.

Impact

Winner detection can be manipulated.

Tools Used

VsCode

Recommended Mitigation Steps

Redesign ThrusterTreasure to be more like state machine that doesn't allow to add more tickets when winner section period started.

Assessed type

Error

c4-judge commented 8 months ago

0xleastwood marked the issue as satisfactory

flowercrimson commented 8 months ago

Contest readme says MEV is out of scope. Also, Blast uses a centralized sequencer to select tx.

MEV attacks are out of scope for this audit.

jooleseth commented 8 months ago

Don't think the MEV part is the issue, and the reason we check 0 is by design because we may not want to set the smaller prizes for a given round, but we will always at least have the larger prizes.

Also the winning ticket number is selected based off the current ticket supply, so if a user enters tickets after the winning tickets are drawn, which should be done atomically by a helper contract, it shouldn't be affected by whether a user newly enters at any new time

c4-sponsor commented 8 months ago

jooleseth (sponsor) disputed

0xleastwood commented 8 months ago

As per sponsor, ticket numbers are based on current ticket supply, so new tickets generated after winner generation would not end up being eligible anyway.

c4-judge commented 8 months ago

0xleastwood marked the issue as unsatisfactory: Invalid