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

12 stars 7 forks source link

Attacker can withdraw Tokens from the Reserve #163

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#L335-L342

Vulnerability details

Impact

Attacker can drain tokens from the reserve.

Proof of Concept

This is due to setDrawManager() function having no access control implemented

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

    emit DrawManagerSet(_drawManager);
  }

An attacker can override the address set in the constructor as drawManager to his own address via the setDrawManager() function that has no access control and bypass the access control on withdrawReserve() function. Thereby draining the funds from the reserve.

 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

Add access control to setDrawManager() function.

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 changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory