Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

settleLongPremium will not collect long premium from option buyer #376

Closed
c4-bot-7 opened this issue Apr 22, 2024 · 3 comments
Closed

settleLongPremium will not collect long premium from option buyer #376

c4-bot-7 opened this issue Apr 22, 2024 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-497 🤖_98_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@c4-bot-7
Copy link
Contributor

Lines of code

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

Vulnerability details

Issue Description

Within PanopticPool, options sellers are allowed to call PanopticPool::settleLongPremium to settle all unpaid premium for long legs of a given buyer position.

The PanopticPool::settleLongPremium function is supposed to calculate the current long premium owed to the buyer using the s_options leg accumulator, then it must collect that the owed premium from the buyer collaterals by calling CollateralTracker::exercise function, and finally, the function will update the storage mapping settledTokens which represents the per-chunk accumulator for tokens owed to options sellers (which will make the premium available for sellers of that chunk).

The issue that it's present in the PanopticPool::settleLongPremium function is that it will mint collateral shares to the buyer instead of burning them from his balance and this is due to the fact that the calculated realizedPremia passed to CollateralTracker::exercise is a positive value. To understand this, let's first look at the function code snippet below:

function settleLongPremium(
    TokenId[] calldata positionIdList,
    address owner,
    uint256 legIndex
) external {
    _validatePositionList(owner, positionIdList, 0);

    TokenId tokenId = positionIdList[positionIdList.length - 1];

    if (tokenId.isLong(legIndex) == 0 || legIndex > 3) revert Errors.NotALongLeg();

    (, int24 currentTick, , , , , ) = s_univ3pool.slot0();

    LeftRightUnsigned accumulatedPremium;
    {
        (int24 tickLower, int24 tickUpper) = tokenId.asTicks(legIndex);

        uint256 tokenType = tokenId.tokenType(legIndex);
        (uint128 premiumAccumulator0, uint128 premiumAccumulator1) = SFPM.getAccountPremium(
            address(s_univ3pool),
            address(this),
            tokenType,
            tickLower,
            tickUpper,
            currentTick,
            1
        );
        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)));

        //@audit realizedPremia is position so exercise will mint collateral shares to buyer instead of burning them

        // 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());

        bytes32 chunkKey = keccak256(
            abi.encodePacked(
                tokenId.strike(legIndex),
                tokenId.width(legIndex),
                tokenId.tokenType(legIndex)
            )
        );
        // commit the delta in settled tokens (all of the premium paid by long chunks in the tokenIds list) to storage
        s_settledTokens[chunkKey] = s_settledTokens[chunkKey].add(
            LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(realizedPremia)))
        );

        emit PremiumSettled(owner, tokenId, realizedPremia);
    }

    // ensure the owner is solvent (insolvent accounts are not permitted to pay premium unless they are being liquidated)
    _validateSolvency(owner, positionIdList, NO_BUFFER);
}

As it can be seen, the realizedPremia which represents the long premium owed by the buyer is calculated using the following formula (for both token 0 & 1):

realizedPremia = (accumulatedPremium * liquidity) / 2**64

If we look at the code above, we can notice that accumulatedPremium is of type LeftRightUnsigned, meaning that it's a positive number (>= 0), and the chunk liquidity is also a positive number (type uint256). This means that the resultant realizedPremia value will also be a positive number as it's the multiplication of two positive numbers.

As we explained above, to collect the owed premium from the buyer collaterals, the CollateralTracker::exercise function is called. This function's logic implies that if the provided realizedPremia value is positive, then we must mint new collateral shares to the user, and if realizedPremia is negative, then we should burn from his balance instead, as shown below:

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;

        //@audit will inverse the realizedPremium sign here
        // 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) {
            //@audit settleLongPremium will always run this because provided realizedPremium > 0
            // if user must receive tokens, mint them
            uint256 sharesToMint = convertToShares(uint256(-tokenToPay));
            _mint(optionOwner, sharesToMint);
        }

        ...
    }
}

So because the realizedPremia value calculated in settleLongPremium is positive and is given to CollateralTracker::exercise function as a positive value, this later will mint new collateral shares to the buyer instead of burning them from his balance (realizedPremium > 0 ==> tokenToPay=-realizedPremium < 0).

This issue means that whenever the PanopticPool::settleLongPremium function is called, the options sellers of that leg chunks being settled will suffer a fund loss as even though the settledTokens accumulator will be updated correctly with the long premium, the actual long premium funds will not be collected from the buyer and made available for the sellers; instead, they will be minted to the buyer as additional collaterals.

Impact

The PanopticPool::settleLongPremium function will not collect long premiums from buyer collateral shares and will instead mint new shares for the buyer, resulting in a loss of funds for all the options sellers.

Tools Used

Manual review, VS Code

Recommended Mitigation

When calling CollateralTracker::exercise in PanopticPool::settleLongPremium, the provided realizedPremia value shoud be made negative as follows:

function settleLongPremium(
    TokenId[] calldata positionIdList,
    address owner,
    uint256 legIndex
) external {
    _validatePositionList(owner, positionIdList, 0);

    TokenId tokenId = positionIdList[positionIdList.length - 1];

    if (tokenId.isLong(legIndex) == 0 || legIndex > 3) revert Errors.NotALongLeg();

    (, int24 currentTick, , , , , ) = s_univ3pool.slot0();

    LeftRightUnsigned accumulatedPremium;
    {
        (int24 tickLower, int24 tickUpper) = tokenId.asTicks(legIndex);

        uint256 tokenType = tokenId.tokenType(legIndex);
        (uint128 premiumAccumulator0, uint128 premiumAccumulator1) = SFPM.getAccountPremium(
            address(s_univ3pool),
            address(this),
            tokenType,
            tickLower,
            tickUpper,
            currentTick,
            1
        );
        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)));
++          .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());

        bytes32 chunkKey = keccak256(
            abi.encodePacked(
                tokenId.strike(legIndex),
                tokenId.width(legIndex),
                tokenId.tokenType(legIndex)
            )
        );
        // commit the delta in settled tokens (all of the premium paid by long chunks in the tokenIds list) to storage
        s_settledTokens[chunkKey] = s_settledTokens[chunkKey].add(
            LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(realizedPremia)))
        );

        emit PremiumSettled(owner, tokenId, realizedPremia);
    }

    // ensure the owner is solvent (insolvent accounts are not permitted to pay premium unless they are being liquidated)
    _validateSolvency(owner, positionIdList, NO_BUFFER);
}

Assessed type

Context

@c4-bot-7 c4-bot-7 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Apr 22, 2024
c4-bot-1 added a commit that referenced this issue Apr 22, 2024
@c4-bot-12 c4-bot-12 added the 🤖_98_group AI based duplicate group recommendation label Apr 22, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as primary issue

@c4-judge
Copy link
Contributor

Picodes marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Apr 30, 2024
@c4-judge
Copy link
Contributor

Picodes marked issue #497 as primary and marked this issue as a duplicate of 497

@c4-judge c4-judge added duplicate-497 and removed primary issue Highest quality submission among a set of duplicates labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-497 🤖_98_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

4 participants