Panoptic - pkqs90's results

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1621-L1640 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1043-L1089

Vulnerability details

Impact

SettleLongPremium is the function intended to settle premiums for long option holders. When called, it should deduct the premium from the option owner's account, but the current implementation adds the premium instead.

Bug Description

Lets see the code for premium calculation. We can see that accumulatedPremium and s_options[owner][tokenId][legIndex] are premium accumulators for calculating the owed amount of premium, and that accumulatedPremium is a LeftRightUnsigned type, which means it must be positive.

The realizedPremia is also positive, because it is calculated by accumulatedPremium * liquidity.

The issue occurs when calling s_collateralToken.exercise(). The realizedPremia that is passed inside should be negative instead of positive, because negative means user pays premia, and positive means user receives premia. The current implementation is incorrect.

PanopticPool.sol

            accumulatedPremium = LeftRightUnsigned
                .wrap(0)
                .toRightSlot(premiumAccumulator0)
                .toLeftSlot(premiumAccumulator1);

            // update the premium accumulator for the long position to the latest value
            // (the entire premia delta will be settled)
            LeftRightUnsigned premiumAccumulatorsLast = s_options[owner][tokenId][legIndex];
            s_options[owner][tokenId][legIndex] = accumulatedPremium;

>           accumulatedPremium = accumulatedPremium.sub(premiumAccumulatorsLast);
        }

        uint256 liquidity = PanopticMath
            .getLiquidityChunk(tokenId, legIndex, s_positionBalance[owner][tokenId].rightSlot())
            .liquidity();

        unchecked {
            // update the realized premia
>           LeftRightSigned realizedPremia = LeftRightSigned
>               .wrap(0)
>               .toRightSlot(int128(int256((accumulatedPremium.rightSlot() * liquidity) / 2 ** 64)))
>               .toLeftSlot(int128(int256((accumulatedPremium.leftSlot() * liquidity) / 2 ** 64)));

            // deduct the paid premium tokens from the owner's balance and add them to the cumulative settled token delta
            s_collateralToken0.exercise(owner, 0, 0, 0, realizedPremia.rightSlot());
            s_collateralToken1.exercise(owner, 0, 0, 0, realizedPremia.leftSlot());

CollateralTracker.sol

   function exercise(
        address optionOwner,
        int128 longAmount,
        int128 shortAmount,
        int128 swappedAmount,
        int128 realizedPremium
    ) external onlyPanopticPool returns (int128) {
        unchecked {
            // current available assets belonging to PLPs (updated after settlement) excluding any premium paid
            int256 updatedAssets = int256(uint256(s_poolAssets)) - swappedAmount;

            // add premium to be paid/collected on position close
>           int256 tokenToPay = -realizedPremium;

            // if burning ITM and swap occurred, compute tokens to be paid through exercise and add swap fees
            int256 intrinsicValue = swappedAmount - (longAmount - shortAmount);

            if ((intrinsicValue != 0) && ((shortAmount != 0) || (longAmount != 0))) {
                // intrinsic value is the amount that need to be exchanged due to burning in-the-money

                // add the intrinsic value to the tokenToPay
                tokenToPay += intrinsicValue;
            }

>           if (tokenToPay > 0) {
                // if user must pay tokens, burn them from user balance (revert if balance too small)
                uint256 sharesToBurn = Math.mulDivRoundingUp(
                    uint256(tokenToPay),
                    totalSupply,
                    totalAssets()
                );
                _burn(optionOwner, sharesToBurn);
>           } else if (tokenToPay < 0) {
                // if user must receive tokens, mint them
                uint256 sharesToMint = convertToShares(uint256(-tokenToPay));
                _mint(optionOwner, sharesToMint);
            }

Proof of Concept

We can also see from unit test test_success_settleLongPremium: The tests checks that after calling settleLongPremium, the assets of Buyer[0] actually increases instead of decreases, which is obviously incorrect.

        assetsBefore0 = ct0.convertToAssets(ct0.balanceOf(Buyers[0]));
        assetsBefore1 = ct1.convertToAssets(ct1.balanceOf(Buyers[0]));

        // collect buyer 1's three relevant chunks
        for (uint256 i = 0; i < 3; ++i) {
            pp.settleLongPremium(collateralIdLists[i], Buyers[0], 0);
        }

        assertEq(
            ct0.convertToAssets(ct0.balanceOf(Buyers[0])) - assetsBefore0,
            33_342,
            "Incorrect Buyer 1 1st Collect 0"
        );

Tools Used

Manual review.

Recommended Mitigation Steps

Take the negative of realizedPremia before calling s_collateralToken.exercise().

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1367-L1391 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L887-L893

Vulnerability details

Impact

The underlying issue is that _validatePositionList() does not check for duplicate tokenIds. Attackers can use this issue to bypass solvency checks, which leads to several impacts:

1. Users can mint/burn/liquidate/forceExercise when they are insolvent.
2. Users can settleLongPremium/forceExercise another user when the other user is insolvent.

This also conflicts a main invariant stated in contest readme: Users should not be allowed to mint/burn options or pay premium if their end state is insolvent.

Bug Description

First, let's see why _validatePositionList does not check for duplicate tokenIds. For a user position hash, the first 8 bits is the length of tokenIds which overflows, last 248 bits is the xor hash. However, we can easily add 256 tokenIds of the same kind to create a duplicate positionHash.

For example: Hash(key0, key1, key2) == Hash(key0, key1, key2, key0, key0, ..., 256 more key0). This way, we can add any tokenId we want while still arriving the same position hash.

PanopticPool.sol

    function _validatePositionList(
        address account,
        TokenId[] calldata positionIdList,
        uint256 offset
    ) internal view {
        uint256 pLength;
        uint256 currentHash = s_positionsHash[account];

        unchecked {
            pLength = positionIdList.length - offset;
        }
        // note that if pLength == 0 even if a user has existing position(s) the below will fail b/c the fingerprints will mismatch
        // Check that position hash (the fingerprint of option positions) matches the one stored for the '_account'
        uint256 fingerprintIncomingList;

        for (uint256 i = 0; i < pLength; ) {
>           fingerprintIncomingList = PanopticMath.updatePositionsHash(
                fingerprintIncomingList,
                positionIdList[i],
                ADD
            );
            unchecked {
                ++i;
            }
        }

        // revert if fingerprint for provided '_positionIdList' does not match the one stored for the '_account'
        if (fingerprintIncomingList != currentHash) revert Errors.InputListFail();
    }

PanopticMath.sol

    function updatePositionsHash(
        uint256 existingHash,
        TokenId tokenId,
        bool addFlag
    ) internal pure returns (uint256) {
        // add the XOR`ed hash of the single option position `tokenId` to the `existingHash`
        // @dev 0 ^ x = x

        unchecked {
            // update hash by taking the XOR of the new tokenId
            uint248 updatedHash = uint248(existingHash) ^
                (uint248(uint256(keccak256(abi.encode(tokenId)))));
            // increment the top 8 bit if addflag=true, decrement otherwise
            return
                addFlag
                    ? uint256(updatedHash) + (((existingHash >> 248) + 1) << 248)
                    : uint256(updatedHash) + (((existingHash >> 248) - 1) << 248);
        }
    }

Then, let's see how duplicate ids can bypass solvency check. The solvency check is in _validateSolvency(), which is called by all the user interaction functions, such as mint/burn/liquidate/forceExercise/settleLongPremium. This function first checks for a user passed in positionIdList (which we already proved can include duplicates), then calls _checkSolvencyAtTick() to calculate the balanceCross (collateral balance) and thresholdCross (required collateral) for all tokens.

The key is the collateral balance includes the premium that is collected for each of the positions. For most of the positions, the collected premium should be less than required collateral to keep this position open. However, if a position has been open for a long enough time, the fees it accumulated may be larger than required collateral.

For this kind of position, we can duplicate it 256 times (or multiple of 256 times, as long as gas fee is enough), and make our collateral balance grow faster than required collateral. This can make a insolvent account "solvent", by duplicating the key tokenId multiple times.

    function _validateSolvency(
        address user,
        TokenId[] calldata positionIdList,
        uint256 buffer
    ) internal view returns (uint256 medianData) {
        // check that the provided positionIdList matches the positions in memory
>       _validatePositionList(user, positionIdList, 0);
        ...
        // Check the user's solvency at the fast tick; revert if not solvent
        bool solventAtFast = _checkSolvencyAtTick(
            user,
            positionIdList,
            currentTick,
            fastOracleTick,
            buffer
        );
        if (!solventAtFast) revert Errors.NotEnoughCollateral();

        // If one of the ticks is too stale, we fall back to the more conservative tick, i.e, the user must be solvent at both the fast and slow oracle ticks.
        if (Math.abs(int256(fastOracleTick) - slowOracleTick) > MAX_SLOW_FAST_DELTA)
            if (!_checkSolvencyAtTick(user, positionIdList, currentTick, slowOracleTick, buffer))
                revert Errors.NotEnoughCollateral();
    }

    function _checkSolvencyAtTick(
        address account,
        TokenId[] calldata positionIdList,
        int24 currentTick,
        int24 atTick,
        uint256 buffer
    ) internal view returns (bool) {
        (
            LeftRightSigned portfolioPremium,
            uint256[2][] memory positionBalanceArray
        ) = _calculateAccumulatedPremia(
                account,
                positionIdList,
                COMPUTE_ALL_PREMIA,
                ONLY_AVAILABLE_PREMIUM,
                currentTick
            );

        LeftRightUnsigned tokenData0 = s_collateralToken0.getAccountMarginDetails(
            account,
            atTick,
            positionBalanceArray,
            portfolioPremium.rightSlot()
        );
        LeftRightUnsigned tokenData1 = s_collateralToken1.getAccountMarginDetails(
            account,
            atTick,
            positionBalanceArray,
            portfolioPremium.leftSlot()
        );

        (uint256 balanceCross, uint256 thresholdCross) = _getSolvencyBalances(
            tokenData0,
            tokenData1,
            Math.getSqrtRatioAtTick(atTick)
        );

        // compare balance and required tokens, can use unsafe div because denominator is always nonzero
        unchecked {
            return balanceCross >= Math.unsafeDivRoundingUp(thresholdCross * buffer, 10_000);
        }
    }

CollateralTracker.sol

    function getAccountMarginDetails(
        address user,
        int24 currentTick,
        uint256[2][] memory positionBalanceArray,
        int128 premiumAllPositions
    ) public view returns (LeftRightUnsigned tokenData) {
        tokenData = _getAccountMargin(user, currentTick, positionBalanceArray, premiumAllPositions);
    }

    function _getAccountMargin(
        address user,
        int24 atTick,
        uint256[2][] memory positionBalanceArray,
        int128 premiumAllPositions
    ) internal view returns (LeftRightUnsigned tokenData) {
        uint256 tokenRequired;

        // if the account has active options, compute the required collateral to keep account in good health
        if (positionBalanceArray.length > 0) {
            // get all collateral required for the incoming list of positions
            tokenRequired = _getTotalRequiredCollateral(atTick, positionBalanceArray);

            // If premium is negative (ie. user has to pay for their purchased options), add this long premium to the token requirement
            if (premiumAllPositions < 0) {
                unchecked {
                    tokenRequired += uint128(-premiumAllPositions);
                }
            }
        }

        // if premium is positive (ie. user will receive funds due to selling options), add this premum to the user's balance
        uint256 netBalance = convertToAssets(balanceOf[user]);
        if (premiumAllPositions > 0) {
            unchecked {
                netBalance += uint256(uint128(premiumAllPositions));
            }
        }

        // store assetBalance and tokens required in tokenData variable
        tokenData = tokenData.toRightSlot(netBalance.toUint128()).toLeftSlot(
            tokenRequired.toUint128()
        );
        return tokenData;
    }

Now we have shown how to bypass the _validateSolvency(), we can bypass all related checks. Listing them here:

1. Burn options #1, #2
2. Mint options #1
3. Force exercise #1, #2
4. SettleLongPremium #1

Proof of Concept

In this report, we will only prove the core issue: duplicate tokenIds are allowed, and won't craft complicated scenarios for relevant impacts.

Add the following test code in PanopticPool.t.sol, we can see that passing 257 of the same token ids can still work for mintOptions().

    function test_duplicatePositionHash(
        uint256 x,
        uint256[2] memory widthSeeds,
        int256[2] memory strikeSeeds,
        uint256[2] memory positionSizeSeeds,
        uint256 swapSizeSeed
    ) public {
        _initPool(x);

        (int24 width, int24 strike) = PositionUtils.getOTMSW(
            widthSeeds[0],
            strikeSeeds[0],
            uint24(tickSpacing),
            currentTick,
            0
        );

        (int24 width2, int24 strike2) = PositionUtils.getOTMSW(
            widthSeeds[1],
            strikeSeeds[1],
            uint24(tickSpacing),
            currentTick,
            0
        );
        vm.assume(width2 != width || strike2 != strike);

        populatePositionData([width, width2], [strike, strike2], positionSizeSeeds);

        // leg 1
        TokenId tokenId = TokenId.wrap(0).addPoolId(poolId).addLeg(
            0, 1, isWETH, 0, 0, 0, strike, width
        );

        TokenId[] memory posIdList = new TokenId[](257);
        for (uint i = 0; i < 257; ++i) {
            posIdList[i] = tokenId;
        }
        pp.mintOptions(posIdList, positionSizes[0], 0, 0, 0);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Add a check in _validatePositionList that the length is shorter than MAX_POSITIONS (32).

Assessed type

Invalid Validation

The bot is programmed to identify accounts that become liquidatable, flagging any account that falls below the required collateral thresholds.

Upon identifying a liquidatable account, the bot triggers an alert. Liquidators can then engage by interacting directly with the smart contracts governing the pool.