AddLiquidity and RemoveLiquidity missing slippage protection

Summary

Functions _mintPosition() and _removeLiquidity() missing slippage protection.

Vulnerability Detail

When addling liquidity to the main and alternative positions, function _mintPosition() sets the amount0Min and amount1Min to 0.

   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
        });

     ...//

    }

As Uniswap V3 docs highlight: https://docs.uniswap.org/contracts/v3/guides/providing-liquidity/mint-a-position#calling-mint

We set amount0Min and amount1Min to zero for the example - but this would be a vulnerability in production. A function calling mint with no slippage protection would be vulnerable to a frontrunning attack designed to execute the mint call at an inaccurate price.

Same issue happens when removes liquidity from the main and alternative positions:

function _removeLiquidity() private {
 ...//
      if (liquidity > 0) {
            decreaseLiquidityParams = INftPositionManager.DecreaseLiquidityParams({
                tokenId: positionMain.nftId,
                liquidity: liquidity,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            });
      }
  ...//

   if (liquidityAlt > 0) {
            decreaseLiquidityParams = INftPositionManager.DecreaseLiquidityParams({
                tokenId: positionAlt.nftId,
                liquidity: liquidityAlt,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            });
    }

...//

 }

The amount0Min and amount1Min are set to 0.

Impact

If the user's transaction suffers from frontrunning, a much smaller amount of position can be minted. When an MEV bot frontruns the removal of liquidity, much smaller amounts of amount0Min and amount1Min are released.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommend do not hardcode slippage protection parameter amount0Min and amount1Min to 0 when increase liquidity or decrease liquidity.

Duplicate of #8

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

Naresh - AddLiquidity and RemoveLiquidity missing slippage protection #70