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

12 stars 7 forks source link

`setDrawManager` lack of two-step role transfer #177

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#L299#L306

Vulnerability details

Impact

if transfer the role to a none-exist address , the contract need to re-deploy. It is recommended to have 2 steps

Proof of Concept

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

    emit DrawManagerSet(_drawManager);
  }

Tools Used

vscode

Recommended Mitigation Steps

It is recommended to implement a two-step role transfer where the role recipient is set and then the recipient has to claim that role to finalise the role transfer

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Insufficient quality