Premium payed by long option holders may be locked in PanopticPool

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1974 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1650-L1652

Vulnerability details

Impact

Premium payed by long option holders may be locked in PanopticPool.

Bug Description

First, we need to understand how the premium works in PanopticPool.

Short option holders earn premium by: 1. Uniswap fees, 2. Long option holder paid premium.

However, long option holders pay premium when long options are closed, or by a manual settleLongPremium() call. If a short option is closed before ALL the corresponding long options (minted after the short option) have paid their premiums, the short option holder won't receive the full premium they're due (unless there were unclaimed premium before). That is why short option holders are expected to call settleLongPremium() for some long options to increase the available premium before closing their positions.

The issue here is not all short option holders will call settleLongPremium() before closing their positions, as it is very likely someone may simply forget to call it before closing, or choose to do so to save gas. If that happens, the short option holder will leave with some premium unclaimed, despite it having been paid by the long option holders. This unclaimed premium then accumulates within the protocol. If no future short options are created, these funds remain locked in the protocol forever.

Note: This is a medium severity instead of high because the premium would be locked only if no future options are created, which is an assumption. However, if this scenario occurs, it would lead to a loss of funds, which should certainly raise attention.

Proof of Concept

In PanopticPool, s_settledTokens is used to keep track of the amount of collected uniswap fees and paid long option premium. We will slightly modify the unit test test_success_settledPremiumDistribution, to show how premium paid by long option holders are locked within the protocol.

In this unit test, the following steps happen:

1. Alice mints 500000 short, Bob mints 250000 short, Charlie mints 250000 short
2. Alice mints 44468 long, Bob mints 44468 long
3. Swap to accumulate uniswap fees
4. Bob burns 250000 short (Important: This short option burn does NOT gain any long option premiums, because they are not paid yet, and will be left in the protocol)
5. Bob mints 250000 short
6. Bob burns 250000 long (Bob pays the long premium)
7. Alice burns 500000 short (Alice gets part of long premium)
8. Alice burns 250000 long (Alice pays the long premium)
9. Charlie burns 250000 short (Charlie gets all long premium)
10. Bob burns 250000 short (Since this position is opened on step 5, after fee accumulation, it doesn't get any premium)

Modify Misc.t.sol to burn all positions:

    function test_success_settledPremiumDistribution() public {
        ...
         assertEq(
             ct1.convertToAssets(ct1.balanceOf(Charlie)) - assetsBefore1,
             275_000_008,
             "Incorrect Charlie Delta 1"
         );
+
+        // Bob burns the rest of his short position.
+        vm.startPrank(Bob);
+        pp.burnOptions($posIdList[1], new TokenId[](0), 0, 0);
     }

Modify PanopticPool.sol to see console logs:

    function _updateSettlementPostBurn(
        address owner,
        TokenId tokenId,
        LeftRightUnsigned[4] memory collectedByLeg,
        uint128 positionSize,
        bool commitLongSettled
    ) internal returns (LeftRightSigned realizedPremia, LeftRightSigned[4] memory premiaByLeg) {
            ...
             // update settled tokens in storage with all local deltas
             s_settledTokens[chunkKey] = settledTokens;

+            console.log("UPDATED s_settledTokens[chunkKey]=", s_settledTokens[chunkKey].rightSlot());
+
             unchecked {
                 ++leg;
             }
         }
     }
 }

Running the unit test, we can see it logs UPDATED s_settledTokens[chunkKey]= 41667 when all positions are closed, which means premium is locked in PanopticPool.

Tools Used

Foundry

Recommended Mitigation Steps

A simple way to mitigate this would be to create a function in PanopticPool to move the extra s_settledTokens back to CollateralTracker (have it tracked in s_poolAssets). The extra premium could be calculated by: s_settledTokens - (grossPremiumCurrent - grossPremiumLast) * totalLiquidity.

Assessed type

Token-Transfer

[c4-judge]

Picodes marked the issue as primary issue

[dyedm1]

True, this is the intended design. Those tokens are not completely useless - they can serve as temporary liquidity for option sellers to withdraw without needing to call settleLongPremium, and could compensate for haircut tokens (allowing them to be withdrawn) if a haircut in that chunk occured. Don't see a failure case here (where to send the tokens is a subjective question).

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[pkqs90]

Hi @Picodes, I'd like to defend my issue.

The leftover tokens, as the report states, in most use case scenarios would serve as liquidity for future option sellers or haircut tokens. However, that is to assume this pool would be used and run forever. If the pool starts with users initially engaging but then gradually dies out, it is almost certain that some tokens will remain trapped in the pool.

I understand that the sponsors may not want to fix this issue, but I think this issue itself should be a medium severity because it would indeed cause lock of funds. The mitigation would be allowing an admin to pull out these tokens to the collateral tracker when they think this pool is dead.

[Picodes]

@pkqs90 I still think this falls within QA as these tokens are indeed "lost" but do not really belong to anyone as they have been forfeited, so there is no real "loss of funds" here. I see the fact that these tokens cannot be saved as a consequence of the permissionless design