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

12 stars 7 forks source link

Unrestricted `DrawManager` Assignment #244

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

The setDrawManager function in the provided contract allows anyone to change the draw manager address without any access control or restrictions. This lack of protection could potentially lead to unauthorized individuals or malicious users assigning themselves as the draw manager, potentially compromising the contract's functionality or security.

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

The function takes an address _drawManager as an argument and assigns it as the new draw manager. However, it does not include any access control checks or restrictions, allowing any caller to change the draw manager. This means that any user, including malicious actors, can call this function and set themselves as the draw manager.

Impact

An unauthorized individual can assume control of the draw manager role, which may have critical permissions or responsibilities within the contract. They can manipulate draws, alter prize distributions, or disrupt the contract's intended functionality, potentially leading to financial loss or other undesirable outcomes.

Proof of Concept

An attacker can exploit this vulnerability by deploying the contract and subsequently calling the setDrawManager function with their desired address as the argument:

PrizePool prizePool = new PrizePool();
prizePool.setDrawManager(msg.sender); // Attacker sets themselves as the draw manager

Tools Used

manual

Recommended Mitigation Steps

The contract already includes a modifier called onlyDrawManager, which can be utilized to restrict access to the 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 marked the issue as satisfactory