sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

0x73696d616f - `Claimable` is vulnerable to return gas bomb attacks, DoSing claiming for all other winners #101

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 3 months ago

0x73696d616f

medium

Claimable is vulnerable to return gas bomb attacks, DoSing claiming for all other winners

Summary

Claimer::claimPrizes() loops through a list of _winners and _prizeIndices to claim, but they may all be DoSed by a single winner returning a gas bomb attack in the Claimer hook.

Vulnerability Detail

Claimer::claimPrizes() receives the list of winners and prize indices and in Claimer::_claim() claims the prize for each winner by calling Vault::claimPrize(). It does so inside a try/catch statement to avoid getting DoSed by failed claims.

Vault inherits Claimable, which calls hooks set by users before and after claiming the prize from the PrizePool, forwarding only a specific amount of gas, HOOK_GAS to prevent attackers from making the transaction revert by running out of gas.

However, attackers can perform a return gas bomb attack and make the transaction run out of gas anyway, as the hook is called via _hooks[_winner].implementation.beforeClaimPrize{ gas: HOOK_GAS }, which returns the _hookData as bytes memory and can run out of gas while copying into memory.

Impact

DoSed claiming for all winners in the Claimer::claimPrizes() transaction and failure of the user that triggered the function to retrieve claiming fees.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-vault/src/abstract/Claimable.sol#L87 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-claimer/src/Claimer.sol#L93-L94

Tool used

Manual Review

Vscode

Recommendation

Call the hooks using ExcessivelySafeCall, which protects against gas bomb attacks. The fix is essentially limiting the return data the hook may send.

Duplicate of #163