code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/models/Pool.sol#L348

Vulnerability details

Impact

Admin can set Pool to emergency state. But there is no possibility to unset it. As result pool will be not possible to use anymore.

Proof of Concept

Pool.adminAction allows owner to set Pool to emergency mode. In emergency mode it's only possible to call adminAction and removeLiquidity functions because both of them use checkReentrancy(true, true) check that allows to call in emergency mode. https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/models/Pool.sol#L339-L354

    function adminAction(uint256 action, uint16 val, address recipient) external {
        checkReentrancy(true, true);
        // ************************ reentrancy ************************
        require(msg.sender == factory.owner());
        if (action == ACTION_SET_PROTOCOL_FEES) {
            setProtocolFeeRatio(val);
        } else if (action == ACTION_CLAIM_PROTOCOL_FEES_A || action == ACTION_CLAIM_PROTOCOL_FEES_B) {
            claimProtocolFees(recipient, action == ACTION_CLAIM_PROTOCOL_FEES_A);
        } else if (action == ACTION_EMERGENCY_MODE) {
            state.status = EMERGENCY;
            return;
        }

        // ************************ reentrancy ************************
        state.status &= ~LOCKED;
    }

But there is no any function that allows owner to unset emergency. As result if emergency risk was mitigated or admin set emergency mistakenly, it's not possible to use Pool anymore. And all LP should withdraw their liquidity. Also it will not be possible to create same Pool using it's Factory as such address already exists and there is no ability to remove it.

I believe that unsetting of emergency state should be added.

Tools Used

VsCode

Recommended Mitigation Steps

Add function to unset emergency mode.

c4-judge commented 1 year ago

kirk-baird marked the issue as primary issue

gte620v commented 1 year ago

Not a bug. This is the intended behavior.

c4-sponsor commented 1 year ago

gte620v marked the issue as sponsor disputed

kirk-baird commented 1 year ago

This is the intended behavior to lock the pool in the case of emergency.

Since no funds are at risk as users are still allowed to withdraw their liquidity via removeLiquidity() I'm going to mark this issue as invalid.