Rhaydden - The `getAmountsForLiquidity` function in LiquidityAmounts.sol is not implemented correctly

[sherlock-admin2]

Rhaydden

high

The getAmountsForLiquidity function in LiquidityAmounts.sol is not implemented correctly

Summary

The function getAmountsForLiquidity() is used to compute the quantity of token0 and token1 value to add to the position given a amount of liquidity. These quantities depend on the amount of liquidity, the current pool prices and the prices at the tick boundaries. Actually, getAmountsForLiquidity() uses the sqrt prices instead of the ticks, but it doesn't matter because they are equivalent since each tick represents a sqrt price.

Albeit, 3 cases exist:

• The current tick is outside the range from the left, this means only token0 should be added.
• The current tick is within the range, this means both token0 and token1 should be added.
• The current tick is outside the range from the right, this means only token1 should be added.

Vulnerability Detail

The issue on the implementation is on the first case, which is coded as follows:

 if (sqrtRatioX96 <= sqrtRatioAX96) {
            amount0 = getAmount0ForLiquidity(
                sqrtRatioAX96,
                sqrtRatioBX96,
                liquidity
            );

As seen above, the implementation indicates that if the current price is equal to the price of the lower tick, it means that it is outside of the range and hence only token0 should be added to the position.

But for the UniswapV3 implementation, the current price must be lower in order to consider it outside:

  if (_slot0.tick < params.tickLower) {
                // current tick is below the passed range; liquidity can only become in range by crossing from left to
                // right, when we'll need _more_ token0 (it's becoming more valuable) so user must provide it
                amount0 = SqrtPriceMath.getAmount0Delta(
                    TickMath.getSqrtRatioAtTick(params.tickLower),
                    TickMath.getSqrtRatioAtTick(params.tickUpper),
                    params.liquidityDelta
                );
            } 

In the discord channel, one of the members of the dev team said the libraries are from uniswap:

Impact

When the current price is equal to the left boundary of the range, the uniswap pool will request both token0 and token1, but Beefy will only request from the user token0 so the pool will lose some token1 if it has enough to cover it.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/utils/LiquidityAmounts.sol#L155-L160

Tool used

Manual Review

Recommendation

- if (sqrtRatioX96 <= sqrtRatioAX96) {
+ if (sqrtRatioX96 < sqrtRatioAX96) {
            amount0 = getAmount0ForLiquidity(
                sqrtRatioAX96,
                sqrtRatioBX96,
                liquidity
            );

[MirthFutures]

Can you write a poc of this issue?

[MirthFutures]

Function is identical to uniswap -> https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/LiquidityAmounts.sol