sherlock-audit / 2024-05-pooltogether-judging

13 stars 8 forks source link

0x73696d616f - `Claimer::_claim()` should reserve gas to finish execution, or it may revert unexpectedly, losing fees and gas for the claimer bot #131

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

0x73696d616f

medium

Claimer::_claim() should reserve gas to finish execution, or it may revert unexpectedly, losing fees and gas for the claimer bot

Summary

Claimer::_claim() calls Vault::claimPrize() inside a try/catch statement and forwards all available gas (63/64 actually), which may end up reverting due to OOG.

Vulnerability Detail

Bots can claim on behalf of other users by using the Claimer smart contract. However, it always tries to claim prizes for a certain winner and prize index, regardless of the amount of gas left. This is very dangerous as it can lead to failed claims if any of the winners implements a hook with non deterministic gas costs (they may change since the transaction was simulated to when it is mined). Thus, claimer bots may have their claim transaction fail due to this, not claiming fees and incurring significant gas costs.

Impact

Failed claim auction mechanism, leading to lost fees and gas costs by reverting due to OOG.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-claimer/src/Claimer.sol#L162

Tool used

Manual Review

Vscode

Recommendation

The maximum amount of gas that can be used when claiming a prize should be precomputed, as well as the remaining gas required to finish execution in Claimer::_claim(). This way, a claimer bot will never fail to claim as if the gas limit is reached, claiming will just stop, instead of reverting and DoSing all other previous claims.

Duplicate of #163