PrizePool -> Winners wouldn't be able to claim prize correctly in `claimPrize` function

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/abstract/TieredLiquidityDistributor.sol#L476

Vulnerability details

Impact

In PrizePool.sol vaults can call claimPrize() function to claim prizes for winners. The claimPrize calls _getTier(_tier, numberOfTiers) function to get back the prizeSize but there is a problem in _getTier function, lets see:

  function _getTier(uint8 _tier, uint8 _numberOfTiers) internal view returns (Tier memory) {
    Tier memory tier = _tiers[_tier];
    uint16 _lastClosedDrawId = lastClosedDrawId;
    if (tier.drawId != _lastClosedDrawId) {
      tier.drawId = _lastClosedDrawId;
      tier.prizeSize = uint96(
        _computePrizeSize(
          _tier,
          _numberOfTiers,
          fromUD34x4toUD60x18(tier.prizeTokenPerShare),
          fromUD34x4toUD60x18(prizeTokenPerShare)
        )
      );
    }
    return tier;
  }

As you can see it calls _computePrizeSize function and downcast the returned result but since it returns uint256 by downcasting it to uint96 it's possible to result in incorrect number if the value overflow (When downcasting from one type to another, Solidity will not revert but overflows). So this means there is possibilty prizeSize value result in incorrect prize size and vaults can't claim the prizes correctly for winners.

Tools Used

Manual Review

Recommended Mitigation Steps

Check whether the result of prizeSize exceeds type(uint96).max or not, if so revert the tx. I recommend to use SafeCast by OpenZeppelin for casting diffrent types safely.

I also found some other downcastings in the code base but since they or pegged to vault share values and you already check for overflows, there is no problem.

Assessed type

Under/Overflow

[asselstine]

Hmm; looks like the Tier struct has an extra 16 bits available to pack. Will increase prizeSize type to uint112 and add safe casting.

[c4-sponsor]

asselstine marked the issue as sponsor confirmed

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[Picodes]

Downgrading to Med as there is no proof of impact or discussion about the chances of this occurring in the report

[c4-judge]

Picodes marked the issue as satisfactory

[asselstine]

Come to think of it, since prizes are in POOL and it's supply is 10m with 18 decimals, then prizes can never be more than ~1e25

Anyway, fixed here https://github.com/GenerationSoftware/pt-v5-prize-pool/pull/16