No slippage check on position minting

Summary

amount0Min & amount1Min are hardcoded to 0, when a position is minted, which may result in unfavourable position being minted and loss of funds.

    function _mintPosition(int24 _tickLower, int24 _tickUpper, uint256 _amount0, uint256 _amount1, bool _mainPosition) private {
        INftPositionManager.MintParams memory mintParams = INftPositionManager.MintParams({
            token0: lpToken0,
            token1: lpToken1,
            tickSpacing: _tickDistance(),
            tickLower: _tickLower,
            tickUpper: _tickUpper,
            amount0Desired: _amount0,
            amount1Desired: _amount1,
            amount0Min: 0,
            amount1Min: 0,
            recipient: address(this),
            deadline: block.timestamp,
            sqrtPriceX96: 0
        });

Vulnerability Detail

This is a serious issue, because onlyCalmPeriod may not be enough under certain circumstances. For example, if twapPeriod is low and it is profitable for the attacker to manipulate the pool for that amount of time, he would be able to lose stakers funds by opening a new position, which liquidity for token0 and token1 = 0. Combined with the fact that there is no effective deadline set, validators could delay the mint transaction and execute it later with less favorable terms.

Impact

Loss of funds

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L314-L317

Tool used

Manual Review

Recommendation

Implement an oracle, and an amount deviation check when you try to mint the position

Duplicate of #8

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

EgisSecurity - No slippage check on position minting #89