0xboriskataa - Deposit function doesn't check if strategy is paused

[sherlock-admin4]

0xboriskataa

high

Deposit function doesn't check if strategy is paused

Summary

Deposit function doesn't check if strategy is paused

Vulnerability Detail

The panic() function is used to claim earnings, remove liquidity and allowances and according to the comment above it also pause deposits:

    /**  
     * @notice Remove Liquidity and Allowances, then pause deposits.
     * @param _minAmount0 The minimum amount of token0 in the strategy after panic.
     * @param _minAmount1 The minimum amount of token1 in the strategy after panic.
     */
    function panic(uint256 _minAmount0, uint256 _minAmount1) public onlyManager {
        _claimEarnings();
        _removeLiquidity();
        _removeAllowances();
        _pause();

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

The issue is that even though _pause() is called in panic() the deposit() function doesn't check if the protocol is in a paused state:

    /// @notice Called during deposit to add all liquidity back to their positions. 
    function deposit() external onlyCalmPeriods {
        _onlyVault();

        if (!initTicks) {
            _setTicks();
            initTicks = true;
        }

        // Add all liquidity
        _addLiquidity();

        (uint256 bal0, uint256 bal1) = balances();

        // TVL Balances after deposit
        emit TVL(bal0, bal1);
    }

Impact

Protocols usually have a pause functionality to protect against attacks and have time to make decisions etc. However users will still be able to deposit in periods they shouldn't be able to.

Code Snippet

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

Tool used

Manual Review

Recommendation

Check if protocol is paused in the deposit() function. There are two ways to do this:

    /// @notice Called during deposit to add all liquidity back to their positions. 
    function deposit() external onlyCalmPeriods {
        _onlyVault();
+       if(_isPaused()) revert PausedState();

        if (!initTicks) {
            _setTicks();
            initTicks = true;
        }

        // Add all liquidity
        _addLiquidity();

        (uint256 bal0, uint256 bal1) = balances();

        // TVL Balances after deposit
        emit TVL(bal0, bal1);
    }

    /// @notice Called during deposit to add all liquidity back to their positions. 
-   function deposit() external onlyCalmPeriods {
+   function deposit() external onlyCalmPeriods whenNotPaused{
        _onlyVault();

        if (!initTicks) {
            _setTicks();
            initTicks = true;
        }

        // Add all liquidity
        _addLiquidity();

        (uint256 bal0, uint256 bal1) = balances();

        // TVL Balances after deposit
        emit TVL(bal0, bal1);
    }

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

z3s commented:

80

_karanel commented:

_addLiquidity reverts this function if strategy is paused