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

12 stars 7 forks source link

Incorrect Calculation of Tier Odds in `getTierOdds` Function. #275

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Risk Breakdown: The incorrect calculation of tier odds can have several implications:

  1. The incorrect odds can lead to imbalances in the prize distribution among different tiers. Some tiers may receive more or fewer prizes than intended, impacting the fairness of the system.

  2. Inaccurate prize distributions can erode user trust in the platform. Users may perceive the system as unfair or unreliable, leading to a negative impact on the platform's reputation.

  3. If the incorrect odds result in significant imbalances in prize distributions, participants may experience financial losses if they expected a different probability of winning.

Proof of Concept

TierCalculationLib.sol#L17-L27

  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))));
  }

The getTierOdds function calculates the odds of a tier occurring for a single draw. However, there is a bug in the calculation of the exponentiation. The line E.pow(_k.mul(sd(int8(_tier) - (int8(_numberOfTiers) - 1)))); should be E.pow(_k.mul(sd(int8(_tier) - int8(_numberOfTiers) + 1)));. The incorrect calculation could lead to incorrect odds for each tier, resulting in inaccurate prize distributions.

To prove the bug, we can use a simple example. Let's assume _tier = 2, _numberOfTiers = 3, and _grandPrizePeriod = 10. We can calculate the expected odds manually:

_k = sd(1).div(sd(int16(10))).ln().div(sd((-1 * int8(3) + 1))) = ...

_expectedOdds = E.pow(_k.mul(sd(int8(2) - (int8(3) - 1)))) = ...

By fixing the calculation, we should get the expected odds. Let's demonstrate this:

_k = sd(1).div(sd(int16(10))).ln().div(sd((-1 * int8(3) + 1))) = ...

_fixedOdds = E.pow(_k.mul(sd(int8(2) - int8(3) + 1))) = ...

Implementation of the proof:

pragma solidity 0.8.17;

import { E, SD59x18, sd, unwrap, toSD59x18, fromSD59x18, ceil } from "prb-math/SD59x18.sol";

contract TierCalculationLibProof {
  function calculateExpectedOdds() external pure returns (SD59x18) {
    SD59x18 _k = sd(1).div(sd(int16(10))).ln().div(sd((-1 * int8(3) + 1)));
    return E.pow(_k.mul(sd(int8(2) - (int8(3) - 1))));
  }

  function calculateFixedOdds() external pure returns (SD59x18) {
    SD59x18 _k = sd(1).div(sd(int16(10))).ln().div(sd((-1 * int8(3) + 1)));
    return E.pow(_k.mul(sd(int8(2) - int8(3) + 1)));
  }
}

References:

Tools Used

None

Recommended Mitigation

See PoC Please.

Assessed type

Math

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Insufficient quality