No slippage protection for AutoCompound::execute::increaseLiquidity.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/AutoCompound.sol#L159-L172

Vulnerability details

Impact

In the AutoCompound execute function an increaseLiquidity call is done to Uniswap with 0 for amount0Min and amount1Min, which allows for a 100% slippage tolerance.

This means the caller could lose up to 100% of their tokens due to slippage.

Proof of Concept

AutoCompound: execute

            // deposit liquidity into tokenId
            if (state.maxAddAmount0 > 0 || state.maxAddAmount1 > 0) {
                _checkApprovals(state.token0, state.token1);

                (, state.compounded0, state.compounded1) = nonfungiblePositionManager.increaseLiquidity(
                    INonfungiblePositionManager.IncreaseLiquidityParams(
                        params.tokenId, state.maxAddAmount0, state.maxAddAmount1, 0, 0, block.timestamp
                    )
                );

From the Uniswap increaseLiquidity documentation:

This example assumes the contract already has custody of the NFT. We cannot change the boundaries of a given liquidity position using the Uniswap v3 protocol; increaseLiquidity can only increase the liquidity of a position.

In production, amount0Min and amount1Min should be adjusted to create slippage protections.

    /// @notice Increases liquidity in the current range
    /// @dev Pool must be initialized already to add liquidity
    /// @param tokenId The id of the erc721 token
    /// @param amount0 The amount to add of token0
    /// @param amount1 The amount to add of token1
    function increaseLiquidityCurrentRange(
        uint256 tokenId,
        uint256 amountAdd0,
        uint256 amountAdd1
    )
        external
        returns (
            uint128 liquidity,
            uint256 amount0,
            uint256 amount1
        )
    {
        INonfungiblePositionManager.IncreaseLiquidityParams memory params =
            INonfungiblePositionManager.IncreaseLiquidityParams({
                tokenId: tokenId,
                amount0Desired: amountAdd0,
                amount1Desired: amountAdd1,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            });

        (liquidity, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity(params);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

These values should be set >0 to protect users.

Assessed type

Uniswap

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as duplicate of #198

[c4-judge]

jhsagd76 marked the issue as unsatisfactory: Out of scope