PoolTogether - Tripathi's results

Lines of code

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

Vulnerability details

Impact

setDrawManager offers Attacker to frontrun the tx which can lock drawManager role. User will have to do costly redeployment which again prone to frontrun attack if he doesn't specify drawManager role in constructor and leads to DOS .

scenarios

1. if drawManager has not been set in constructor [DOS] this will lead to DOS as for every deployment Attacker can frontrun the setDrawManager. This will lock the drawManager role and so prizePool

2. As in constructor there is no address check for address so invalid address will cost redeployment

Proof of Concept

see Impact

File: src/PrizePool.sol

258 constructor(
    ConstructorParams memory params
  )
    TieredLiquidityDistributor(
      params.numberOfTiers,
      params.tierShares,
      params.canaryShares,
      params.reserveShares
    )
  {
    if (unwrap(params.smoothing) >= unwrap(UNIT)) {
      revert SmoothingGTEOne(unwrap(params.smoothing));
    }
    prizeToken = params.prizeToken;
    twabController = params.twabController;
    smoothing = params.smoothing;
    claimExpansionThreshold = params.claimExpansionThreshold;
    drawPeriodSeconds = params.drawPeriodSeconds;
    _lastClosedDrawStartedAt = params.firstDrawStartsAt;

    drawManager = params.drawManager;
    if (params.drawManager != address(0)) {
      emit DrawManagerSet(params.drawManager);
    }
282  }

299  function setDrawManager(address _drawManager) external {
    if (drawManager != address(0)) {
      revert DrawManagerAlreadySet();
    }
    drawManager = _drawManager;

306    emit DrawManagerSet(_drawManager);
  }

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

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

Tools Used

Manual

Recommended Mitigation Steps

set the drawManager role to deployer(msg.sender) in constructor and then add a option to change setdrawManager where deployer can set drawManager to any address

Assessed type

Access Control

Lines of code

https://github.com/PaulRBerg/prb-math/blob/df27d3d12ce12153fb166e1e310c8351210dc7ba/src/sd59x18/Math.sol#L596-L609 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/DrawAccumulatorLib.sol#L390-L399

Vulnerability details

Impact

Protocol is highly dependent on the computation of area under the curve c(d) = −t ∗ ln(α)∗α ^ d which uses PRB-Math's Pow() function to calculate . Recently It has been found this pow() function can give inconsistent values and break the computation.

PRBMath's pow() function takes two signed integer (say a and b) and returns a**b A proper implementation of power function states that : for any a >= b and x >= 0 , a ** x >= b ** x

Simplifying this further as (a>=b)= ((a/b)>=1) and so (a/b)**x >=1 [x>=0] this pow() function doesn't follow this and gives inconsistent values.

This Mainly affects integrate function of DrawAccumulatorLib.sol which integrate from a lower value higher value for the exponential weighted average

Proof of Concept

File: prb-math/src/sd59x18/Math.sol

596: function pow(SD59x18 x, SD59x18 y) pure returns (SD59x18 result) {
    int256 xInt = x.unwrap();
    int256 yInt = y.unwrap();

    if (xInt == 0) {
        result = yInt == 0 ? UNIT : ZERO;
    } else {
        if (yInt == uUNIT) {
            result = x;
        } else {
            result = exp2(mul(log2(x), y));
        }
    }
609: }

Test Function

function testPow() public {
uint128 high = 505456470057136353;
uint128 low = 505456461792312955;
assert(high > low);
SD59x18 exp = UD2x18.wrap(31414735).intoSD59x18();
SD59x18 highPow = high.intoSD59x18().pow(exp);
SD59x18 lowPow = low.intoSD59x18().pow(exp);
assert(SD59x18.unwrap(highPow) < SD59x18.unwrap(lowPow));
}

Root cause described by a senior watson - "in the exp2() function, bit masks are applied to multiply the result by root(2,2^-i) for each position i with a value of 1. However at Common.sol#L170, the value is incorrectly included as 0xFF00000000 instead of 0xFF000000 "

Tools Used

Manual , Foundry Testing

Recommended Mitigation Steps

Ensure that calculation is accurate by adjusting the comparison value in exp2() function

Assessed type

Math