sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

0xaghas - Missing Access Control in setPoolActive Function and its Internal Implementation #900

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xaghas

high

Missing Access Control in setPoolActive Function and its Internal Implementation

The setPoolActive function, which allows toggling the status of a pool between active and inactive, lacks an appropriate access control modifier. This omission is also observed in its internal implementation, _setPoolActive.

Vulnerability Detail

Both the setPoolActive function and its internal counterpart _setPoolActive lack access control modifiers, allowing any external entity to change the pool's active status. Such a critical function, especially one related to toggling the active state of a pool, should have proper restrictions to prevent unauthorized modifications.

Impact

Without access control, malicious or unintended actors can exploit this vulnerability to toggle the pool's status, potentially disrupting normal operations, causing unexpected behaviors, or leading to a denial of service where users cannot interact with the pool as expected. This poses not only a functional risk but also undermines trust in the system.

Code Snippet

RFPSimpleStrategy.sol https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/rfp-simple/RFPSimpleStrategy.sol#L216C1-L222C6

BaseStrategy.sol https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/BaseStrategy.sol#L272C1-L279C6

Tool used

Manual Review

Recommendation

Implement an access control modifier such as onlyPoolManager (if only a designated pool manager should be able to change the status) or another appropriate modifier based on your system's design. Apply this modifier to setPoolActive function.