search

sherlock-audit / 2024-05-pooltogether-judging

2 stars 0 forks source link

0x73696d616f - `Claimer::claimPrizes()` and `Claimer::computeTotalFees()` do not validate the intended `drawId`, which may incur significant costs for claimer bots #151

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

0x73696d616f

medium

Claimer::claimPrizes() and Claimer::computeTotalFees() do not validate the intended drawId, which may incur significant costs for claimer bots

Summary

Claimer::claimPrizes() and Claimer::computeTotalFees() do not validate the intended drawId, which will punish bots that claim prizes at the very of the drawId, but their transaction only gets included after the drawId ends, not claiming any prize, but still incurring gas costs.

Vulnerability Detail

Claimer::claimPrizes() and Claimer::computeTotalFees() get the elapsed time based on block.timestamp - prizePool.lastAwardedDrawAwardedAt(). The problem with this approach is that the draw may be over for the intended drawId, but the next draw has not yet been awarded, so the elapsed time will actually be bigger than a full draw duration and bots expect to receive more fees.

However, whenever bots submit a Claimer::claimPrizes() transaction a few blocks before the end of the draw, the transaction may be delayed to be included by just a few blocks and the right drawId would be over. It will not revert initially due to the _minFeePerClaim check as the elapsed time is based on block.timestamp - prizePool.lastAwardedDrawAwardedAt(), as mentioned above, so it will return more than the specified minimum fee, but the draw may be over.

Thus, such users will incur significant costs without receiving any fee as each Vault::claimPrize() transaction will revert, but spend all the gas regardless (up to a little more than 150k gas per call, or 15_000_000 * 20*10^(-9) == 0.3 ETH if there are 100 winners and a gas price of 20 gwei).

It is a possible scenario that claim auctions are only profitable at the very end of the draw duration as gas prices may spike, prize pools may be small for a certain draw and/or several start auctions may fail (each one taking at most an auctionDuration).

Impact

Bots incur very significant costs as it is not checked if the correct drawId is being claimed, so each Vault::claimPrize() will revert.

Code Snippet

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

Tool used

Manual Review

Vscode

Recommendation

Receive the drawId as argument and validate that it matches the _lastAwardedDrawId of the PrizePool.

sherlock-admin4 commented 1 month ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

low__ auctions are designed for start and finish draw to happen early and if not the next auction target price will correct this

nevillehuang commented 1 month ago

Invalid, sponsor comments:

  • This is a gas optimization at best
  • Either way, the call reverts