code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Claimer.claimPrizes can be frontrunned in order to make losses for the claim bot #115

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L60-L83

Vulnerability details

Impact

Claimer.claimPrizes can be frontrunned in order to make losses for the claim bot

Proof of Concept

All prizes in the system should be claimed during draw period. Anyone can call claimPrizes function for any winner and prize. But the idea of protocol is to incentivize claim bots to do all claims instead of users. These should benefit from batching claims together in 1 call.

Also Claimer contract is using vrgda which can increase/decrease fee for the claimer, depending of amount of already claimed prizes. That actually means that in case if not enough prizes are claimed in some point of time, then fee will be increased to incentivize more bots.

So as bots will be trying to batch a lot of prize claims together, that means that they will spent a lot on gas. Malicious actor can see which prizes bot is going to claim and frontrun him with last prize is the list. As result claiming will fail and bot will face losses.

Another reason, why malicious actor can do that is to make vrgda to provide bigger fees. So he can create a bot that will block claims of another bots with big amount of claims in the batch, in order to wait and get better prices and claim those prize by himself and receive fees.

Tools Used

VsCode

Recommended Mitigation Steps

I guess that in case if price is already claimed, then you can return early from the PrizePool.claimPrize function and emit some event. In this case bot will not receive fee for already claimed prize and his tx will continue with other prizes in the batch.

Assessed type

Error

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor confirmed

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-judge commented 1 year ago

Picodes marked the issue as selected for report

Picodes commented 1 year ago

I'll keep medium severity as the sponsor confirmed with its label that it was valuable to him, but I think we could also justify this being low as in such cases it is up to the bot to use a private rpc or some other solution to avoid being frontran.

asselstine commented 1 year ago

Fixed in

https://github.com/GenerationSoftware/pt-v5-claimer/pull/8 https://github.com/GenerationSoftware/pt-v5-vault/pull/22 https://github.com/GenerationSoftware/pt-v5-prize-pool/pull/23