`_claimExpansionThreshold` may not work.

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L781

Vulnerability details

Impact

_computeNextNumberOfTiers() Logic problem that may cause _claimExpansionThreshold to invalid.

Proof of Concept

To add tiers two conditions need to be met:

The number of tiers is increased when:

     the number of claimed standard prizes exceeds the claim threshold % of the expected number of standard prizes.
     the number of claimed canary prizes exceeds the expected number of canary prizes

The current implementation code is as follows:

  function _computeNextNumberOfTiers(uint8 _numTiers) internal view returns (uint8) {
    UD2x18 _claimExpansionThreshold = claimExpansionThreshold;

@>  uint8 _nextNumberOfTiers = largestTierClaimed + 2; // canary tier, then length
    _nextNumberOfTiers = _nextNumberOfTiers > MINIMUM_NUMBER_OF_TIERS
      ? _nextNumberOfTiers
      : MINIMUM_NUMBER_OF_TIERS;

    if (_nextNumberOfTiers >= MAXIMUM_NUMBER_OF_TIERS) {
      return MAXIMUM_NUMBER_OF_TIERS;
    }

    // check to see if we need to expand the number of tiers
@>  if (
      _nextNumberOfTiers >= _numTiers &&
      canaryClaimCount >=
      fromUD60x18(
        intoUD60x18(_claimExpansionThreshold).mul(_canaryPrizeCountFractional(_numTiers).floor())
      ) &&
      claimCount >=
      fromUD60x18(
        intoUD60x18(_claimExpansionThreshold).mul(toUD60x18(_estimatedPrizeCount(_numTiers)))
      )
    ) {
      // increase the number of tiers to include a new tier
      _nextNumberOfTiers = _numTiers + 1;
    }
@>  return _nextNumberOfTiers;
  }

According to the above implementation Assumption: current numberOfTiers = 3 if a claim canary tier has been executed, claimPrize(_tier = 2) After that become: canaryClaimCount = 1 largestTierClaimed = 2 _nextNumberOfTiers = largestTierClaimed + 2 = 4

According to the above implementation, even if the condition in if (_claimExpansionThreshold) is not satisfied, it will still return _nextNumberOfTiers, i.e.: return 4;.

So if someone maliciously executes claimPrize(_tier = canary tier) one time every draw, regardless of whether _claimExpansionThreshold is met, then the number of tiers will be increased by 1 every draw. The _claimExpansionThreshold limit is invalidated.

Tools Used

Recommended Mitigation Steps

  function _computeNextNumberOfTiers(uint8 _numTiers) internal view returns (uint8) {
    UD2x18 _claimExpansionThreshold = claimExpansionThreshold;

    uint8 _nextNumberOfTiers = largestTierClaimed + 2; // canary tier, then length
    _nextNumberOfTiers = _nextNumberOfTiers > MINIMUM_NUMBER_OF_TIERS
      ? _nextNumberOfTiers
      : MINIMUM_NUMBER_OF_TIERS;

    if (_nextNumberOfTiers >= MAXIMUM_NUMBER_OF_TIERS) {
      return MAXIMUM_NUMBER_OF_TIERS;
    }

    // check to see if we need to expand the number of tiers
    if (
      _nextNumberOfTiers >= _numTiers &&
      canaryClaimCount >=
      fromUD60x18(
        intoUD60x18(_claimExpansionThreshold).mul(_canaryPrizeCountFractional(_numTiers).floor())
      ) &&
      claimCount >=
      fromUD60x18(
        intoUD60x18(_claimExpansionThreshold).mul(toUD60x18(_estimatedPrizeCount(_numTiers)))
      )
    ) {
      // increase the number of tiers to include a new tier
      _nextNumberOfTiers = _numTiers + 1;
-   }
+   }else if(_nextNumberOfTiers > _numTiers) {
+      _nextNumberOfTiers = _numTiers
+   }

    return _nextNumberOfTiers;
  }

Assessed type

Context

[c4-sponsor]

asselstine requested judge review

[asselstine]

Duplicate of https://github.com/code-423n4/2023-07-pooltogether-findings/issues/105

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked issue #332 as primary and marked this issue as a duplicate of 332