Ineffective slippage check due to Incorrect order of operation in the panic() function .

Summary

The function panic() in the StrategyPassiveManagerVelodrome contract handles situations by claiming earnings, removing liquidity, removing allowances, and pausing the contract. However, the order of operations within the function can cause the slippage check to be ineffective, allowing critical actions to be executed even if slippage conditions are not met.

Vulnerability Detail

The issue arises from the placement of the slippage check after the _claimEarnings(), _removeLiquidity(), _removeAllowances(), and _pause() calls. This sequence means that the function performs these actions before verifying if the balance conditions _minAmount0 and _minAmount1 are satisfied.

Below is the panic() function :

    function panic(uint256 _minAmount0, uint256 _minAmount1) public onlyManager {
        _claimEarnings();
        _removeLiquidity();
        _removeAllowances();
        _pause();

//@audit :
        (uint256 bal0, uint256 bal1) = balances();
        if (bal0 < _minAmount0 || bal1 < _minAmount1) revert TooMuchSlippage();
    }

As you can see the slippage check occurs after critical operations.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L807

Impact

If the balances of bal0 and bal1 are below the specified minimum amounts _minAmount0 and _minAmount1, since the slippage check occurs after critical operations, these actions may still execute, leading to undesirable consequences such as incomplete or incorrect state changes,

Tool used

Manual Review

Recommendation


function panic(uint256 _minAmount0, uint256 _minAmount1) public onlyManager {
    // Get current balances before taking any action
 + (uint256 bal0, uint256 bal1) = balances();
 + if (bal0 < _minAmount0 || bal1 < _minAmount1) revert TooMuchSlippage();

    _claimEarnings();
    _removeLiquidity();
    _removeAllowances();
    _pause();

//move the check above
 - (uint256 bal0, uint256 bal1) = balances();
 - if (bal0 < _minAmount0 || bal1 < _minAmount1) revert TooMuchSlippage();

}

sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

air_0x - Ineffective slippage check due to Incorrect order of operation in the panic() function . #60