If all the liquidity for a specific position is transferred from Uniswap, that liquidity will not generate fees.

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L1050-L1059
https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L1394-L1420

Vulnerability details

Impact

There are two types of liquidity: one remains in Uniswap, and the other is transferred from Uniswap by users.
SFPM tracks owed fees for the liquidity moved from Uniswap as if it were still in Uniswap.
This is crucial because protocols utilizing SFPM can access these owed fees and request fees from users moving liquidity from Uniswap.

However, if all liquidity for certain positions is shifted from Uniswap, these specific liquidities will not generate fees.
Users utilizing this liquidity are consequently exempt from fees in this scenario.

Proof of Concept

All fee calculations are predicated on the assumption that the net liquidity is not zero.
When minting or burning tokens, we only update the owed and gross premia when the net liquidity is non-zero.
https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L1050-L1059

if (currentLiquidity.rightSlot() > 0) {
     _totalCollected = _collectAndWritePositionData(
           _liquidityChunk,
           _univ3pool,
           currentLiquidity,
           positionKey,
           _moved,
           isLong
      );
}

The same principle applies in the getAccountPremium function.
https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L1394-L1420

if (netLiquidity != 0) {
    ...            
}

However, a net liquidity of zero does not imply that a position lacks liquidity; this situation arises when all liquidity has been moved from Uniswap by users.

Consider the following example: a user creates a short call through Panoptic for [L1, U1] and deposits X liquidity into Uniswap.
The user expects to receive at least fees from Uniswap.
Another user creates a long call through Panoptic for [L1, U1] and transfers X liquidity from Uniswap.
The net liquidity for [L1, U1] is now zero, and Uniswap will not generate fees for this.
Consequently, the liquidity will not generate any fees either.

In practical terms, this means that the user who created the short call cannot receive any fees, and the user who created the long call can use that liquidity without incurring fees.

The PoC test is as below:

function test_getAccountPremium(
        uint256 x,
        uint256 widthSeed,
        int256 strikeSeed,
        uint256 positionSizeSeed,
        uint256 swapSize) public 
    {
        _initPool(x);
        (int24 width, int24 strike) = PositionUtils.getInRangeSW(
            widthSeed,
            strikeSeed,
            uint24(tickSpacing),
            currentTick
        );
        populatePositionData(width, strike, positionSizeSeed);

        // Alice creates a short position and adds liquidity to Uni V3.
        uint256 tokenId = uint256(0).addUniv3pool(poolId).addLeg(0, 1, isWETH, 0, 1, 0, strike, width);
        sfpm.mintTokenizedPosition(tokenId, uint128(positionSize), TickMath.MIN_TICK, TickMath.MAX_TICK);

        // And she moves that liquidity from Uniswap by creating a long position.
        uint256 tokenId_1 = uint256(0).addUniv3pool(poolId).addLeg(0, 1, isWETH, 1, 1, 0, strike, width);
        sfpm.mintTokenizedPosition(tokenId_1, uint128(positionSize), TickMath.MIN_TICK, TickMath.MAX_TICK);

        // Bob also creates the same short position and adds liquidity to Uni V3.
        changePrank(Bob);
        uint256 tokenId_2 = uint256(0).addUniv3pool(poolId).addLeg(0, 1, isWETH, 0, 1, 0, strike, width);
        sfpm.mintTokenizedPosition(tokenId_2, uint128(positionSize), TickMath.MIN_TICK, TickMath.MAX_TICK);
        
        swapSize = bound(swapSize, 10 ** 15, 10 ** 19);

        // To add fees to the Uni Pool, execute the following swaps.
        router.exactInputSingle(
            ISwapRouter.ExactInputSingleParams(
                isWETH == 0 ? token0 : token1,
                isWETH == 1 ? token0 : token1,
                fee,
                Bob,
                block.timestamp,
                swapSize,
                0,
                0
            )
        );
        router.exactOutputSingle(
            ISwapRouter.ExactOutputSingleParams(
                isWETH == 1 ? token0 : token1,
                isWETH == 0 ? token0 : token1,
                fee,
                Bob,
                block.timestamp,
                swapSize - (swapSize * fee) / 1_000_000,
                type(uint256).max,
                0
            )
        );

        (, currentTick, , , , , ) = pool.slot0();

        // poke uniswap pool
        changePrank(address(sfpm));
        pool.burn(tickLower, tickUpper, 0);
        changePrank(Alice);

        (uint128 premiumToken0, uint128 premiumtoken1) = sfpm.getAccountPremium(
            address(pool),
            Alice,
            1,
            tickLower,
            tickUpper,
            currentTick,
            0
        );
        // Alice doesn't have any fees, but the moved liquidity from Uniswap should still generate fees as if it were still in Uni V3.
        assertEq(premiumToken0, 0);
        assertEq(premiumtoken1, 0);

        (uint128 premiumToken0_Bob, uint128 premiumtoken1_Bob) = sfpm.getAccountPremium(
            address(pool),
            Bob,
            1,
            tickLower,
            tickUpper,
            currentTick,
            0
        );
        // Bob has fees generated from his position.
        assertGt(premiumToken0_Bob, 0);
        assertGt(premiumtoken1_Bob, 0);
}

Tools Used

Manual

Recommended Mitigation Steps

It is essential to account for scenarios where the net liquidity is zero.
At this point, the spread variable, represented as v * R / N, is undefined when N is 0.
Additionally, the equation feesCollected = feesGrowthX128 * (T-R) is not valid when N (which is equal to T−R) is 0.
One possible solution is to track the fee growth amount instead of s_accountFeesBase.

Assessed type

Math

[c4-judge]

Picodes marked the issue as duplicate of #362

[c4-judge]

Picodes marked the issue as selected for report

[c4-sponsor]

dyedm1 (sponsor) disputed

[dyedm1]

It's important to note that this account premium calculation is not actually used anywhere within the SFPM, and is an opinionated calculation exposed for optional use by protocols such as Panoptic to price options.

With that being said, part of the opinionated definition of that calculation includes a multiplier on the premium for option buyers based on the utilization on a given chunk. As is illustrated by this graph of the relationship between the utilization and the premium multiplier, there is a vertical asymptote at 100% utilization. Thus, it is expected that protocols using this feature will cap utilization at a certain percentage (for example, Panoptic allows a maximum utilization of 90%).

The long premium calculations at a utilization of 100% (where netLiquidity=0) cannot be performed because the multiplier approaches infinity and would be undefined at that utilization. In that situation, it makes the most sense to forego the premium calculation. Protocols who use this calculation should understand this behavior and either implement a utilization cap or accept that premium will be 0 at utilization=100%

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[Picodes]

Considering the maths of the multiplier and the fact that these functions are only helpers within the SFPM itself, there is no bug here so these reports are valuable but only highlighting an edge case protocols should be aware of. I'll downgrade to QA.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as not selected for report