Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dimah7 - Super pools can't be paused, in case of an emergency #360

Closed
sherlock-admin4 opened this issue Aug 24, 2024 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Aug 24, 2024

dimah7

Medium

Super pools can't be paused, in case of an emergency

Summary

Super pools functions can't be paused, in case of an emergency

Vulnerability Detail

As can be seen in SuperPool.sol, there is a pausing/unpausing function implemented, with the idea to freeze the functionality:

/// @notice Toggle pause state of the SuperPool
    function togglePause() external onlyOwner {
        if (Pausable.paused()) Pausable._unpause();
        else Pausable._pause();
    }

The problem is that this logic is inefficient. Based on OZ's Pausable.sol code a whenNotPaused modifier is required in order to freeze the functions successfully when the pool is in paused state:

OZ::Pausable.sol:

 modifier whenNotPaused() {
        _requireNotPaused();
        _;
    }


function _requireNotPaused() internal view virtual {
        if (paused()) {
            revert EnforcedPause();
        }
    }

For example the correct logic is implemented in the PositionManager contract, but lacks in the super pool:

                                                                                 !!!!!!!!!!!!!
function process(address position, Action calldata action) external nonReentrant whenNotPaused {
        _process(position, action);
        if (!riskEngine.isPositionHealthy(position)) revert PositionManager_HealthCheckFailed(position);
    }

Impact

  • Impact: High, in case of an emergency the admin can't freeze the protocol. The worst outcome would be funds to be stolen
  • Likelihood: Low, as it requires a breach in the protocol's code to happen
  • Overall: Medium

Code Snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/SuperPool.sol#L20
https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/SuperPool.sol#L164-L167

Tool used

Manual Review

Recommendation

Add the whenNotPaused modifier where necessary

Duplicate of #270

@github-actions github-actions bot closed this as completed Sep 5, 2024
@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. labels Sep 5, 2024
@sherlock-admin4 sherlock-admin4 changed the title Merry Butter Dog - Super pools can't be paused, in case of an emergency dimah7 - Super pools can't be paused, in case of an emergency Sep 15, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant