code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L80 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L618-L629 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1058-L1065 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L473

Vulnerability details

Impact

Within the Vault#claimPrizes(), there is no limitation how many prize indices of each winner stored in the _prizeIndices storage can be claimed in a single transaction.

If some winner has too many prize indices of the winner in the _prizeIndices storage, the transaction will be reverted due to reaching the gas limit in the for-loop in the Vault#claimPrizes().

Proof of Concept

When a claimer claim a prize on behalf of a user, the Claimer#claimPrizes() would be called by a claimer. Within the Claimer#claimPrizes(), the Vault#claimPrizes() would be called like this: https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L80

  /// @notice Allows the call to claim prizes on behalf of others.
  /// @param vault The vault to claim from
  /// @param tier The tier to claim for
  /// @param winners The array of winners to claim for
  /// @param prizeIndices The array of prize indices to claim for each winner (length should match winners)
  /// @param _feeRecipient The address to receive the claim fees
  /// @return totalFees The total fees collected across all successful claims
  function claimPrizes(
    Vault vault,
    uint8 tier,
    address[] calldata winners,
    uint32[][] calldata prizeIndices,
    address _feeRecipient
  ) external returns (uint256 totalFees) {
    ...
    vault.claimPrizes(tier, winners, prizeIndices, feePerClaim, _feeRecipient);  /// @audit
    ...
  }

Within the Vault#claimPrizes(), the Vault#_claimPrize() would be called for every single winner (_winners[w]) and their prize indice (_prizeIndices[w][p]) in the for-loop like this: https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L618-L629

  /**
   * @notice Claim prizes for the `_winners`
   * @dev The caller must be the claimer
   * @param _tier Tier to claim prize for
   * @param _winners Addresses of the winners to claim prizes
   * @param _prizeIndices The prizes to claim for each winner
   * @param _feePerClaim Fee to be charged per prize claim
   * @param _feeRecipient Address that will receive `_claimFee` amount
   * @return uint256 The total prize amounts claimed
   */
  function claimPrizes(
    uint8 _tier,
    address[] calldata _winners,
    uint32[][] calldata _prizeIndices,
    uint96 _feePerClaim,
    address _feeRecipient
  ) external returns (uint256) {
    if (msg.sender != _claimer) revert CallerNotClaimer(msg.sender, _claimer);

    uint totalPrizes;

    for (uint w = 0; w < _winners.length; w++) { /// @audit
      uint prizeIndicesLength = _prizeIndices[w].length;
      for (uint p = 0; p < prizeIndicesLength; p++) {  /// @audit
        totalPrizes += _claimPrize( /// @audit
          _winners[w],  /// @audit
          _tier,
          _prizeIndices[w][p],  /// @audit
          _feePerClaim,
          _feeRecipient
        );
      }
    }
    ...

Within the Vault#_claimPrize(), the PrizePool#claimPrize() would be called like this: https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1058-L1065

  /**
   * @notice Claim prize for `_winner`.
   * @param _winner Address of the winner to claim the prize for
   * @param _tier Tier of the prize
   * @param _prizeIndex The prize index to claim
   * @param _fee Fee to be charged for the claim
   * @param _feeRecipient Address that will receive the fee
   * @return uint256 The total prize amount claimed
   */
  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( /// @audit
      _winner,
      _tier,
      _prizeIndex,
      recipient,
      _fee,
      _feeRecipient
    );
    ...
  }

Within the PrizePool#claimPrize(), the amount of Prize Token would be transferred via the _transfer() like this: https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L473

  function claimPrize(
    address _winner,
    uint8 _tier,
    uint32 _prizeIndex,
    address _prizeRecipient,
    uint96 _fee,
    address _feeRecipient
  ) external returns (uint256) {
    ...
    _transfer(_prizeRecipient, amount);  /// @audit - Prize Token would be sent to the user from the PrizePool
    ...

However, within the Vault#claimPrizes(), there is no limitation how many prize indices of each winner stored in the _prizeIndices storage can be claimed in a single transaction. If some winner has too many prize indices of the winner in the _prizeIndices storage, the transaction will be reverted due to reaching the gas limit in the for-loop in the Vault#claimPrizes().

Tools Used

Foundry

Recommended Mitigation Steps

Within the Vault#claimPrizes(), consider adding a limitation (cap) how many prize indices of each winner stored in the _prizeIndices storage can be claimed in a single transaction.