Lines of code

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

Vulnerability details

[M-01] disrupting program flow and potential loss of data or funds in setDrawManager function

The setDrawManager function sets the drawManager address to the input parameter _drawManager and emits an event DrawManagerSet. However, before setting the drawManager, it checks if it is not already set by verifying if it is not equal to address(0).

The risk here is that if the drawManager is already set to a non-zero address, the function will revert with an error message DrawManagerAlreadySet(). This can cause disruption in the normal flow of the program and may cause loss of data or funds if the function is called at an inappropriate time or in an inappropriate context

The impact

the setDrawManager function can be significant. If the function is called at an inappropriate time or in an inappropriate context, it can cause disruption in the normal flow of the program and may result in loss of data or funds.

For example, if the function is called after the drawManager has already been set, it will revert with an error message and prevent any further changes to the drawManager. This can cause delays in the execution of other functions that rely on the drawManager and may lead to unexpected behavior or errors.

In addition, if the input parameter _drawManager is not validated properly, it can result in unintended consequences such as assigning the wrong address to the drawManager, which can cause further issues down the line.

Overall, the impact of this risk can be significant and should be mitigated through proper checks, validations, and authorization mechanisms.

Proof of concept

the setDrawManager function can be demonstrated by creating a test scenario where the function is called multiple times with different input parameters.

For example, we can create a test contract that calls the setDrawManager function with a non-zero address, then calls it again with a different non-zero address. We can then verify that the drawManager is set to the second address and that the event DrawManagerSet is emitted.

Next, we can call the function again with the first non-zero address and verify that it reverts with the error message DrawManagerAlreadySet(). This will demonstrate that the function can cause disruption in the normal flow of the program and may prevent further changes to the drawManager.

Finally, we can test the function with an input parameter that is not a valid address, such as a string or an integer, and verify that it reverts with an error message or causes unintended consequences. This will demonstrate that proper input validation is necessary to prevent unintended consequences and potential loss of data or funds.

Overall, this proof of concept will demonstrate the potential risks and consequences of the setDrawManager function and highlight the importance of proper checks, validations, and authorization mechanisms.

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

    emit DrawManagerSet(_drawManager);
  }

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

Tools Used

VSCode, and Manual

Recommanded steps

It is recommended to add more checks and validations to ensure that the function is called only when necessary and by authorized parties, and that the input parameter is valid and does not cause any unintended consequences.

Use a modifier to restrict access to authorized parties only. This can prevent unauthorized parties from calling the function and causing unintended consequences.

Add input validation to ensure that the input parameter _drawManager is a valid address and not equal to the current drawManager. This can prevent unintended consequences and potential loss of data or funds.

Consider adding a time lock or delay mechanism to prevent the function from being called multiple times within a short period. This can prevent disruption in the normal flow of the program and ensure that changes to the drawManager are deliberate and intentional.

Overall, it is important to consider the specific requirements and context of the setDrawManager function and implement appropriate checks, validations, and authorization mechanisms to ensure its safety and reliability.

Assessed type

Reentrancy

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

disrupting program flow and potential loss of data or funds in setDrawManager function #367