sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

ydlee - Prize winners can set claim hooks to revert `claimPrize` from others to save the claim rewards. #110

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 3 months ago

ydlee

medium

Prize winners can set claim hooks to revert claimPrize from others to save the claim rewards.

Summary

Prize winners can set claim hooks to revert claimPrize from others, and then they claimPrize themselves with 0 claim reward. Winners have the incentive to do so as this will save the claim reward. However, this will discourage other normal claimers to claim prizes.

Vulnerability Detail

The Prize Claimer contract allows anyone to claim prizes on behalf of winners, and earn rewards in doing so. The calling path to claim prizes is Claimer.claimPrizes => Claimer._claim => Claimable.claimPrize => PrizePool.claimPrize. Claimer.claimPrizes will calculate the claim rewards and the rewards will be added to the claim reward recipient's account (L563). Note that the claim rewards are paid out of the winner's prize (L566), then the remaining prize is transferd to the _prizeRecipient (L590).

// Function: PrizePool.claimPrize()

560:    uint256 amount;
561:    if (_claimReward != 0) {
562:      emit IncreaseClaimRewards(_claimRewardRecipient, _claimReward);
563:@>    _rewards[_claimRewardRecipient] += _claimReward;
564:
565:      unchecked {
566:@>      amount = tierLiquidity.prizeSize - _claimReward;
567:      }
568:    } else {
569:      amount = tierLiquidity.prizeSize;
570:    }
571:
572:    // co-locate to save gas
573:    claimCount++;
574:    _totalWithdrawn = SafeCast.toUint128(_totalWithdrawn + amount);
575:    _totalRewardsToBeClaimed = SafeCast.toUint104(_totalRewardsToBeClaimed + _claimReward);
...    // ...
589:    if (amount > 0) {
590:@>    prizeToken.safeTransfer(_prizeRecipient, amount);
591:    }

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/PrizePool.sol#L560-L591

Each account is allowed to set claim hooks that can be called when they win. The hooks are called in Claimable.claimPrize, before or after PrizePool.claimPrize. In such case, a winner can set an after claim hook, and the hook reverts the claimPrize if the claim reward (i.e. _reward) is not 0. The winner listens for this revert event, and then calls Claimer.claimPrizes himself to claim his own prize with 0 claim reward. In such case, the winner can save the claim reward, and gains the total prize.

// Function: Claimable.claimPrize()

100:        uint256 _prizeTotal = prizePool.claimPrize(
101:            _winner,
102:            _tier,
103:            _prizeIndex,
104:            _prizeRecipient,
105:            _reward,
106:            _rewardRecipient
107:        );
108:
109:        if (_hooks[_winner].useAfterClaimPrize) {
110:@>          _hooks[_winner].implementation.afterClaimPrize{ gas: HOOK_GAS }(
111:                _winner,
112:                _tier,
113:                _prizeIndex,
114:                _prizeTotal - _reward,
115:                _prizeRecipient,
116:                _hookData
117:            );
118:        }

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-vault/src/abstract/Claimable.sol#L100-L118

Winners have the incentive to do so as this will save the claim reward. However, this will discourage other normal claimers to claim prizes.

Impact

Winners can set claim hook to revert others' claimPrize to save claim rewards. This discourages other normal claimers to claim prizes. The worst case scenario is that the incentive mechanism may be destroyed.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/PrizePool.sol#L560-L591

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-vault/src/abstract/Claimable.sol#L100-L118

Tool used

Manual Review

Recommendation

Due to the presence of claim hooks, I am unable to devise a good solution to this problem. Perhaps removing the claim hooks could be considered.

Duplicate of #73

sherlock-admin2 commented 2 months ago

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

infect3d commented:

PrizePool.claimPrize will revert of the prize has already been claimed