ydlee - `_getMinLiquidityAmt` does not return the minimum liquidity amount.

[sherlock-admin2]

ydlee

medium

_getMinLiquidityAmt does not return the minimum liquidity amount.

Summary

Function _getMinLiquidityAmt does not return the minimum liquidity amount as expected. This will cause function _extractLiquidity to work incorrectly.

Vulnerability Detail

Function _getMinLiquidityAmt calculates the liquidity amount for amount0 and amount1, and is expected to return the minimum one. However, it currently returns the larger one.

    function _getMinLiquidityAmt(
        int24 tickLower,
        int24 tickUpper
    ) internal pure returns (uint128 minLiquidity) {
        uint128 liquidity0 = LiquidityAmounts.getLiquidityForAmount0(
            TickMath.getSqrtRatioAtTick(tickUpper - 1),
            TickMath.getSqrtRatioAtTick(tickUpper),
            Constants.MINIMUM_EXTRACTED_AMOUNT
        );
        uint128 liquidity1 = LiquidityAmounts.getLiquidityForAmount1(
            TickMath.getSqrtRatioAtTick(tickLower),
            TickMath.getSqrtRatioAtTick(tickLower + 1),
            Constants.MINIMUM_EXTRACTED_AMOUNT
        );
=>      minLiquidity = liquidity0 > liquidity1 ? liquidity0 : liquidity1;
    }

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L107-L122

As a result, in function _extractLiquidity, when load.liquidity is within the interval between liquidity0 and liquidity1, the loan.liquidity < minLiquidityAmt condition will pass (as minLiquidityAmt is the larger one of liquidity0 and liquidity1) and will continue to decrease the liquidity. But it is expected to revert with InvalidBorrowedLiquidityAmount.

            // Check borrowed liquidity validity
=>          uint128 minLiquidityAmt = _getMinLiquidityAmt(cache.tickLower, cache.tickUpper);
=>          if (loan.liquidity > cache.liquidity || loan.liquidity < minLiquidityAmt) {
                revert InvalidBorrowedLiquidityAmount(
                    loan.tokenId,
                    cache.liquidity,
                    minLiquidityAmt,
                    loan.liquidity
                );
            }

            // Calculate borrowed amount
            borrowedAmount += cache.holdTokenDebt;
            // Decrease liquidity and move to the next loan
=>          _decreaseLiquidity(loan.tokenId, loan.liquidity);

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L155-L169

Impact

_extractLiquidity is not working properly when load.liquidity is within the interval between liquidity0 and liquidity1. It should revert, but it continues to decrease the liquidity.

Code Snippet

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L107-L122

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L155-L169

Tool used

Manual Review

Recommendation

Fix the function _getMinLiquidityAmt and return the minimum value of liquidity0 and liquidity1.

-        minLiquidity = liquidity0 > liquidity1 ? liquidity0 : liquidity1;
+        minLiquidity = liquidity0 > liquidity1 ? liquidity1 : liquidity0;

[nevillehuang]

Invalid , there seems to be some misunderstanding with naming, function works correctly and returns the minimum value that can be borrowed i.e. the maximum amount of liquidity for borrowing