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

12 stars 7 forks source link

Anyone can be drawManager and steal funds with withdrawReserve #253

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/PrizePool.sol#L296-L306 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L332-L342

Vulnerability details

Impact

Anyone can call setDrawManager() and be drawManager.

Proof of Concept

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; //@audit here
    if (params.drawManager != address(0)) {
      emit DrawManagerSet(params.drawManager);
    }
  }

If drawManager is not set in the constructor, anyone can call setDrawManager() function and can be drawManager.

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

    emit DrawManagerSet(_drawManager);
  }

After became drawManager, attacker can steal funds with withdrawReserve() function.

function withdrawReserve(address _to, uint104 _amount) external onlyDrawManager {
    if (_amount > _reserve) {
      revert InsufficientReserve(_amount, _reserve);
    }
    _reserve -= _amount;
    _transfer(_to, _amount);
    emit WithdrawReserve(_to, _amount);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Access Control

Assessed type

Access Control

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #356

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory