Users may receive incorrect amounts of liquidity tokens when minting or burning positions.

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L289-L330 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L324-L330 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L328

Vulnerability details

Vulnerability Description

In the getLiquidityChunk function of the PanopticMath, the liquidity calculation for a position leg does not properly handle the case when the asset parameter is set to 1. This can lead to incorrect liquidity amounts being minted or burned in the SFPM contract.

function getLiquidityChunk(
    TokenId tokenId,
    uint256 legIndex,
    uint128 positionSize
) internal pure returns (LiquidityChunk) {
    ...
    if (tokenId.asset(legIndex) == 0) {
        return Math.getLiquidityForAmount0(tickLower, tickUpper, amount);
    } else {
        return Math.getLiquidityForAmount1(tickLower, tickUpper, amount);
    }
}

Impact

The function retrieves the tick range (tickLower and tickUpper) for the leg using tokenId.asTicks(legIndex). It then calculates the liquidity based on the asset parameter encoded in the tokenId.

PanopticMath.sol#L324-L328

uint256 amount = uint256(positionSize) * tokenId.optionRatio(legIndex);
if (tokenId.asset(legIndex) == 0) {
    return Math.getLiquidityForAmount0(tickLower, tickUpper, amount);
} else {
    return Math.getLiquidityForAmount1(tickLower, tickUpper, amount);
}

When the asset parameter of the tokenId is set to 1. In this case, the function incorrectly assumes that the liquidity should be calculated using Math.getLiquidityForAmount1.

According to the comments in the code, when asset is 1, the amount of token1 moved at strike K is K, and the amount of token0 moved is 1. This means that the liquidity calculation should be based on token0, not token1.

This #Line328 Is responsible

return Math.getLiquidityForAmount1(tickLower, tickUpper, amount);

Proof of Concept

The getLiquidityChunk function can lead to incorrect liquidity calculations and potentially impact the protocol and its users.

Scenario

• A user mints an option position with tokenId that has the asset parameter set to 1.
• The option position has a single leg with legIndex 0, tickLower of 50000, tickUpper of 50200, and positionSize of 10.
• The optionRatio for the leg is 1.

PoC

User calls the mintOptions function passing the necessary parameters, including the tokenId with asset set to 1.

Inside the mintOptions function, the _mintInSFPMAndUpdateCollateral function is called, which eventually invokes the mintTokenizedPosition function in the SFPM contract.

The SFPM contract calls the _validateAndForwardToAMM function, which in turn calls the _createPositionInAMM function.

The _createPositionInAMM function loops through each leg of the tokenId and calls the _createLegInAMM function for each leg.

Inside the _createLegInAMM function, the getLiquidityChunk function from the PanopticMath library is called to calculate the liquidity chunk for the leg.

The getLiquidityChunk function receives the following inputs:

• tokenId: The option position ID with asset set to 1.
• legIndex: 0
• positionSize: 10

The function retrieves the tick range: tickLower = 50000 and tickUpper = 50200.

Calculates the amount:

   uint256 amount = uint256(positionSize) * tokenId.optionRatio(legIndex);
   // amount = 10 * 1 = 10

Since tokenId.asset(legIndex) is 1, the function incorrectly uses Math.getLiquidityForAmount1 to calculate the liquidity:

   return Math.getLiquidityForAmount1(tickLower, tickUpper, amount);
   // Incorrectly calculates liquidity based on token1

The Math.getLiquidityForAmount1 function is called with tickLower = 50000, tickUpper = 50200, and amount = 10.

The function calculates the liquidity based on the formula:

    Liquidity = amount1 / (sqrt(upper) - sqrt(lower))

The calculated liquidity is returned to the _createLegInAMM function and subsequently used to mint the liquidity in the Uniswap V3 pool.

Tools Used

Manual audit, VS Code

Recommended Mitigation

Update the liquidity calculation logic to handle the case when tokenId.asset(legIndex) == 1 correctly. Ensure that the correct function (getLiquidityForAmount1) is called and the appropriate parameters are passed.

uint256 amount = uint256(positionSize) * tokenId.optionRatio(legIndex);
if (tokenId.asset(legIndex) == 0) {
    return Math.getLiquidityForAmount0(tickLower, tickUpper, amount);
} else {
    // Calculate liquidity based on token0 when asset is 1
    return Math.getLiquidityForAmount0(tickLower, tickUpper, 1);
}

When asset is 1, the liquidity is calculated using Math.getLiquidityForAmount0 with an amount of 1, as per the expected behavior described in the comments.

Assessed type

Math

[dyedm1]

That comment could be a bit confusing, but the asset is supposed to represent the unit of position sizing. The suggested "correct" sizing path makes no sense (asset=1 does not mean 1 of token0).

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid