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

12 stars 7 forks source link

Tiers odds compuations is not accurate #75

Open code423n4 opened 12 months ago

code423n4 commented 12 months ago

Lines of code

https://github.com/generationsoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L26

Vulnerability details

Impact

Detailed description of the impact of this finding. Tiers odds compuations is not accurate

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. If you will go to wolfram link than equation for tiers can be simplified to this image

So code for ods will become like this

  function getTierOdds(
    uint8 _tier,
    uint8 _numberOfTiers,
    uint16 _grandPrizePeriod
  ) internal pure returns (SD59x18) {
    SD59x18 _k = sd(1).div(sd(int16(_grandPrizePeriod))).ln().div(
      sd((-1 * int8(_numberOfTiers) + 1))
    );

    return E.pow(_k.mul(sd(int8(_tier) - (int8(_numberOfTiers) - 1))));
  }
// new equation
  function getTierOdds2(
    uint8 _tier,
    uint8 _numberOfTiers,
    uint16 _grandPrizePeriod
  ) internal pure returns (SD59x18) {

    return sd(1).div(sd(int16(_grandPrizePeriod))).pow((sd(int8(_tier) - (int8(_numberOfTiers) - 1))).div(sd((-1 * int8(_numberOfTiers) + 1))));
  }

blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L26

Here is an example before and after, its not a huge difference. But seems like real tier odds are slightly less than current this means that protocol will give out prize a little bit more than it should. Above is current formula, below for new formula.

  SD59x18 internal constant TIER_ODDS_1_4 = SD59x18.wrap(19579642462506911);
  SD59x18 internal constant TIER_ODDS_1_4 = SD59x18.wrap(19579642462506911);
  ----------
  SD59x18 internal constant TIER_ODDS_2_4 = SD59x18.wrap(139927275620255366);
  SD59x18 internal constant TIER_ODDS_2_4 = SD59x18.wrap(139927275620255364);
  ----------
  SD59x18 internal constant TIER_ODDS_3_4 = SD59x18.wrap(1000000000000000000);
  SD59x18 internal constant TIER_ODDS_3_4 = SD59x18.wrap(1000000000000000000);
  ----------
  SD59x18 internal constant TIER_ODDS_0_5 = SD59x18.wrap(2739726027397260);
  SD59x18 internal constant TIER_ODDS_0_5 = SD59x18.wrap(2739726027397260);
  ----------
  SD59x18 internal constant TIER_ODDS_1_5 = SD59x18.wrap(11975133168707466);
  SD59x18 internal constant TIER_ODDS_1_5 = SD59x18.wrap(11975133168707465);
  ----------
  SD59x18 internal constant TIER_ODDS_2_5 = SD59x18.wrap(52342392259021369);
  SD59x18 internal constant TIER_ODDS_2_5 = SD59x18.wrap(52342392259021367);
  ----------
  SD59x18 internal constant TIER_ODDS_3_5 = SD59x18.wrap(228784597949733865);
  SD59x18 internal constant TIER_ODDS_3_5 = SD59x18.wrap(228784597949733862);

Tools Used

Recommended Mitigation Steps

Fix a tier odds

Assessed type

Error

c4-sponsor commented 11 months ago

asselstine marked the issue as sponsor confirmed

Picodes commented 11 months ago

Considering the rounding issue is of a few wei, I'll downgrade this to low under the assumption that the error is not significant.

c4-judge commented 11 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

Picodes marked the issue as grade-a

asselstine commented 10 months ago

Fixed in https://github.com/GenerationSoftware/pt-v5-prize-pool/pull/24