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

12 stars 7 forks source link

Denial-of-service attack on "prizes claiming" transaction #373

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1053 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1068

Vulnerability details

The "prizes claiming" transaction can be under a denial-of-service (DoS) attack.

Proof of Concept

The Vault._claimPrize() implements hook triggers (beforeClaimPrize and afterClaimPrize) and calls them if a winner enables one of them.

In this way, an attacker can register hook functions that can make a denial-of-service (DoS) attack on the "prizes claiming" transaction by reverting the transaction, creating a returnbomb attack, or spending all available gas.

    function _claimPrize(
        address _winner,
        uint8 _tier,
        uint32 _prizeIndex,
        uint96 _fee,
        address _feeRecipient
    ) internal returns (uint256) {
        VaultHooks memory hooks = _hooks[_winner];
        address recipient;
        if (hooks.useBeforeClaimPrize) {
@>      recipient = hooks.implementation.beforeClaimPrize(_winner, _tier, _prizeIndex);
        } else {
            recipient = _winner;
        }

        uint prizeTotal = _prizePool.claimPrize(
            _winner,
            _tier,
            _prizeIndex,
            recipient,
            _fee,
            _feeRecipient
        );

        if (hooks.useAfterClaimPrize) {
@>      hooks.implementation.afterClaimPrize(
                _winner,
                _tier,
                _prizeIndex,
                prizeTotal - _fee,
                recipient
            );
        }

        return prizeTotal;
    }

beforeClaimPrize hook: https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1053

afterClaimPrize hook: https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1068

Impact

Since one "prizes claiming" transaction can contain a batch of multiple winners, other legit winners will not be able to receive their prizes.

Of course, the likelihood of this issue might be considered "LOW" (since the attacker must be one of the winners), but the impact of this issue is considered "HIGH".

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend applying the excessivelySafeCall() to avoid the DoS attack via the beforeClaimPrize and afterClaimPrize hook functions.

Assessed type

DoS

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #465

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory