no - StrategyPassiveManagerVelodrome::withdraw missing the _onlyCalmPeriods check

[sherlock-admin3]

no

medium

StrategyPassiveManagerVelodrome::withdraw missing the _onlyCalmPeriods check

Summary

StrategyPassiveManagerVelodrome::withdraw missing the _onlyCalmPeriods check

Vulnerability Detail

function withdraw(uint256 _amount0, uint256 _amount1) external {
        _onlyVault();

        // Liquidity has already been removed in beforeAction() so this is just a simple withdraw.
        if (_amount0 > 0) IERC20Metadata(lpToken0).safeTransfer(vault, _amount0);
        if (_amount1 > 0) IERC20Metadata(lpToken1).safeTransfer(vault, _amount1);

        // After we take what is needed we add it all back to our positions. 
        if (!_isPaused()) _addLiquidity();

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

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

When a withdraw is initiated, BeefyVaultConcLiq::withdraw calls StrategyPassiveMan- StrategyPassiveManagerVelodrome::beforeAction which removes the liquidity. In 4 other places when liquidity has been removed and added, onlyCalmPeriods is always checked immediately before calling _- addLiquidity in deposit(), moveTicks(), setPositionWidth(), unpause().

Impact

Whenever withdrawals occur the newly added liquidity can be based off a stale current tick. The most likely result of this is reduced liquidity provider rewards due to a non-optimal LP position.

Code Snippet

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

Tool used

Manual Review

Recommendation

StrategyPassiveManagerVelodrome::withdraw adds the _onlyCalmPeriods check

Duplicate of #74