AddLiquidity and decreaseLiquidity missing slippage protection

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/LiquidityPosition.sol#L195 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/LiquidityPosition.sol#L258

Vulnerability details

Impact

AddLiquidity and decreaseLiquidity missing slippage protection

Proof of Concept

When user mint NFT add liquidity

user can specify two parameter, params.amount0Min and params.amount1Min

        // mint the position
        (tokenId, liquidity, amount0Minted, amount1Minted) = Base.UNI_POSITION_MANAGER.mint(
            INonfungiblePositionManager.MintParams({
                token0: params.token0,
                token1: params.token1,
                fee: params.fee,
                tickLower: params.tickLower,
                tickUpper: params.tickUpper,
                amount0Desired: params.amount0ToMint,
                amount1Desired: params.amount1ToMint,
                amount0Min: params.amount0Min,
                amount1Min: params.amount1Min,
                recipient: address(this),
                deadline: block.timestamp
            })
        );

if the minted amount is too small, transaction revert in this check in Uniswap position manager when addling liquidity

(amount0, amount1) = pool.mint(
    params.recipient,
    params.tickLower,
    params.tickUpper,
    liquidity,
    abi.encode(MintCallbackData({poolKey: poolKey, payer: msg.sender}))
);

require(amount0 >= params.amount0Min && amount1 >= params.amount1Min, 'Price slippage check');

However, when addling liquidity, the parameter amount0Min and amount1Min is set to 0

// increase liquidity via position manager
(liquidity, amount0Added, amount1Added) = Base.UNI_POSITION_MANAGER.increaseLiquidity(
    INonfungiblePositionManager.IncreaseLiquidityParams({
        tokenId: tokenId,
        amount0Desired: amount0,
        amount1Desired: amount1,
        amount0Min: 0,
        amount1Min: 0,
        deadline: block.timestamp
    })
);

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.

if the user transaction suffer from frontrunning, a much less amount of token can be minted

same issue happens when user decrease liquidity

    function decreaseLiquidity(uint256 tokenId, uint128 liquidity) internal returns (uint256 amount0, uint256 amount1) {
        (amount0, amount1) = Base.UNI_POSITION_MANAGER.decreaseLiquidity(
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: tokenId,
                liquidity: liquidity,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            })
        );
    }

the amonut0 and amount1Min are set to 0

when MEV bot frontruns the decrease liquidity, much less amount0 and amount1 are released

Tools Used

Manual Review

Recommended Mitigation Steps

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

Assessed type

Token-Transfer

[0xleastwood]

This seems valid and serious. Worth adding as user-controlled parameters.

[c4-judge]

0xleastwood marked the issue as primary issue

[wukong-particle]

Agreed. Will add slippage protection when increase/decrease liquidity.

[c4-judge]

0xleastwood marked the issue as selected for report

[wukong-particle]

Add label: sponsor confirmed

[c4-sponsor]

wukong-particle (sponsor) confirmed