0xShoonya - Lack of slippage checks on `StrategyPassiveManagerVelodrome::_mintPosition`

[sherlock-admin2]

0xShoonya

medium

Lack of slippage checks on StrategyPassiveManagerVelodrome::_mintPosition

Summary

The _mintPosition function in StrategyPassiveManagerVelodrome.sol lacks slippage checks. It sets amount0Min and amount1Min to zero, which provides no protection against slippage.

Vulnerability Detail

When the deposit function is called by the vault:

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

_addLiquidity is called internally:

    function _addLiquidity() private {
        _whenStrategyNotPaused();

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

        int24 mainLower = positionMain.tickLower;
        int24 mainUpper = positionMain.tickUpper;
        int24 altLower = positionAlt.tickLower;
        int24 altUpper = positionAlt.tickUpper;

        // Then we fetch how much liquidity we get for adding at the main position ticks with our token balances. 
        uint160 sqrtprice = sqrtPrice();
        uint128 liquidity = LiquidityAmounts.getLiquidityForAmounts(
            sqrtprice,
            TickMath.getSqrtRatioAtTick(mainLower),
            TickMath.getSqrtRatioAtTick(mainUpper),
            bal0,
            bal1
        );

        (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(
            sqrtprice,
            TickMath.getSqrtRatioAtTick(mainLower),
            TickMath.getSqrtRatioAtTick(mainUpper),
            liquidity
        );

        bool amountsOk = _checkAmounts(liquidity, mainLower, mainUpper);

        // Mint or add liquidity to the position. 
        if (liquidity > 0 && amountsOk) {
            _mintPosition(mainLower, mainUpper, amount0, amount1, true);
        }

        (bal0, bal1) = balancesOfThis();

        // Fetch how much liquidity we get for adding at the alternative position ticks with our token balances.
        liquidity = LiquidityAmounts.getLiquidityForAmounts(
            sqrtprice,
            TickMath.getSqrtRatioAtTick(altLower),
            TickMath.getSqrtRatioAtTick(altUpper),
            bal0,
            bal1
        );

        (amount0, amount1) = LiquidityAmounts.getAmountsForLiquidity(
            sqrtprice,
            TickMath.getSqrtRatioAtTick(altLower),
            TickMath.getSqrtRatioAtTick(altUpper),
            liquidity
        );

        // Mint or add liquidity to the position.
        if (liquidity > 0 && (amount0 > 0 || amount1 > 0)) {
            _mintPosition(altLower, altUpper, amount0, amount1, false);
        }

        if (positionMain.nftId != 0) ICLGauge(gauge).deposit(positionMain.nftId);
        if (positionAlt.nftId != 0) ICLGauge(gauge).deposit(positionAlt.nftId);
    }

which further calls _mintPosition to mint a new position for the main or alternative position.

 _mintPosition(altLower, altUpper, amount0, amount1, false);

Issue is inside _mintParams, INFTPositionManager is called with MintParams:

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

        (uint256 nftId,,,) = INftPositionManager(nftManager).mint(mintParams);

        if (_mainPosition) positionMain.nftId = nftId;
        else positionAlt.nftId = nftId;

        IERC721(nftManager).approve(gauge, nftId);
    }

Here, amount0Min and amount1Min are hardcoded as 0 which offers no slippage protection.

Impact

Without slippage checks, there is a risk that funds can be exploited by MEV (Miner Extractable Value) bots. These bots can manipulate the transaction to extract value, leading to potential financial loss for users.

Code Snippet

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

Tool used

Manual Review

Recommendation

Modify the _mintPosition function to allow callers to specify amount0Min and amount1Min values instead of setting them to zero.

Duplicate of #8

[sherlock-admin4]

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

WildSniper commented:

invalid due to impossibility of such attack with tick lower and upper as params