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

haircutPremia will not cover protocol losses using liquidatee long premiums #534

Open
c4-bot-6 opened this issue Apr 22, 2024 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b primary issue Highest quality submission among a set of duplicates Q-09 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_242_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@c4-bot-6
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/libraries/PanopticMath.sol#L768-L858
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1043-L1080
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1122-L1131

Vulnerability details

Issue Description

When the user positions are getting liquidated the PanopticPool::liquidate function will invoke under the hood the PanopticMath::haircutPremia function which will use the liquidatee's owed long premium to cover any eventual protocol losses.

The issue that it's present in the PanopticPool::haircutPremia function is that when trying to exercice the haircut amounts it will mint new collaterals shares to the liquidatee instead of burning them from his existing balance and this is due to the fact that the calculated haircut amounts haircut0 and haircut1 passed to CollateralTracker::exercise are both positive values.

To understand this, let's first look at the function code snippet below:

function haircutPremia(
    address liquidatee,
    TokenId[] memory positionIdList,
    LeftRightSigned[4][] memory premiasByLeg,
    LeftRightSigned collateralRemaining,
    CollateralTracker collateral0,
    CollateralTracker collateral1,
    uint160 sqrtPriceX96Final,
    mapping(bytes32 chunkKey => LeftRightUnsigned settledTokens) storage settledTokens
) external returns (int256, int256) {
    unchecked {
        // get the amount of premium paid by the liquidatee
        LeftRightSigned longPremium;
        for (uint256 i = 0; i < positionIdList.length; ++i) {
            TokenId tokenId = positionIdList[i];
            uint256 numLegs = tokenId.countLegs();
            for (uint256 leg = 0; leg < numLegs; ++leg) {
                if (tokenId.isLong(leg) == 1) {
                    longPremium = longPremium.sub(premiasByLeg[i][leg]);
                }
            }
        }
        // Ignore any surplus collateral - the liquidatee is either solvent or it converts to <1 unit of the other token
        int256 collateralDelta0 = -Math.min(collateralRemaining.rightSlot(), 0);
        int256 collateralDelta1 = -Math.min(collateralRemaining.leftSlot(), 0);
        int256 haircut0;
        int256 haircut1;
        // if the premium in the same token is not enough to cover the loss and there is a surplus of the other token,
        // the liquidator will provide the tokens (reflected in the bonus amount) & receive compensation in the other token
        if (
            longPremium.rightSlot() < collateralDelta0 &&
            longPremium.leftSlot() > collateralDelta1
        ) {
            int256 protocolLoss1 = collateralDelta1;
            (collateralDelta0, collateralDelta1) = (
                -Math.min(
                    collateralDelta0 - longPremium.rightSlot(),
                    PanopticMath.convert1to0(
                        longPremium.leftSlot() - collateralDelta1,
                        sqrtPriceX96Final
                    )
                ),
                Math.min(
                    longPremium.leftSlot() - collateralDelta1,
                    PanopticMath.convert0to1(
                        collateralDelta0 - longPremium.rightSlot(),
                        sqrtPriceX96Final
                    )
                )
            );

            haircut0 = longPremium.rightSlot();
            haircut1 = protocolLoss1 + collateralDelta1;
        } else if (
            longPremium.leftSlot() < collateralDelta1 &&
            longPremium.rightSlot() > collateralDelta0
        ) {
            int256 protocolLoss0 = collateralDelta0;
            (collateralDelta0, collateralDelta1) = (
                Math.min(
                    longPremium.rightSlot() - collateralDelta0,
                    PanopticMath.convert1to0(
                        collateralDelta1 - longPremium.leftSlot(),
                        sqrtPriceX96Final
                    )
                ),
                -Math.min(
                    collateralDelta1 - longPremium.leftSlot(),
                    PanopticMath.convert0to1(
                        longPremium.rightSlot() - collateralDelta0,
                        sqrtPriceX96Final
                    )
                )
            );

            haircut0 = collateralDelta0 + protocolLoss0;
            haircut1 = longPremium.leftSlot();
        } else {
            // for each token, haircut until the protocol loss is mitigated or the premium paid is exhausted
            haircut0 = Math.min(collateralDelta0, longPremium.rightSlot());
            haircut1 = Math.min(collateralDelta1, longPremium.leftSlot());

            collateralDelta0 = 0;
            collateralDelta1 = 0;
        }

        {
            //@audit haircut0 & haircut1 will be positive so we it will mint new shares to liquidatee instead of removing them
            address _liquidatee = liquidatee;
            if (haircut0 != 0) collateral0.exercise(_liquidatee, 0, 0, 0, int128(haircut0));
            if (haircut1 != 0) collateral1.exercise(_liquidatee, 0, 0, 0, int128(haircut1));
        }

        ...
    }
}

As it can be seen, the function will calculates the values of both haircut0 and haircut1 which represents the portion of long premium owed by the liquidatee that must be haircut to cover the protocol losses (this means that this portion of owed long premium will not be given to the seller but instead returned to the collateral pool for PLP).

If we look at the code above, we can notice the following:

  • longPremium is a positive number as is calculated using this formula:
longPremium = longPremium.sub(premiasByLeg[i][leg]);

We know that for long options premiasByLeg is negative and so by using sub (applying -) longPremium we'll be the sum of - premiasByLeg which we'll be positive.

  • collateralDelta0 & collateralDelta1 are both positive numbers (either equal to 0 or above it) which can be deducted from their definition:
int256 collateralDelta0 = -Math.min(collateralRemaining.rightSlot(), 0);
int256 collateralDelta1 = -Math.min(collateralRemaining.leftSlot(), 0);

If collateralRemaining is negative then we'll get a positive number after applying (-) and if its above 0 then the min will return 0.

Knowing this two facts we can concluded that no matter which if statement block is run in the haircutPremia function to calculate haircut0 and haircut1, both their values will be positive as they are the combination (equal or sum) of positive values: (collateralDelta0 or collateralDelta1), (protocolLoss0 or protocolLoss1) and longPremium (and this is quiet evident in the last else block).

As we explained above, to get the owed premium from the liquidatee collaterals, the CollateralTracker::exercise function is called. This function's logic implies that if the provided realizedPremia (represented by haircut0 and haircut1 here) 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 haircutPremia 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 both haircut0 and haircut1 are positive and they represent the realized premia realizedPremia provided to CollateralTracker::exercise function, this later will mint new collateral shares to the liquidatee instead of burning them from his balance (realizedPremium > 0 ==> tokenToPay=-realizedPremium < 0).

This issue means that whenever the haircutPremia function is called in a liquidation call, if there are some protocol losses the function will not haircut from the liquidatee collateral shares balance and will mint him new collateral shares instead, this will result in a protocol loss and will impact all the PLP and other users that deposited collaterals into the collateral tracker which will experience a direct fund loss due to this issue.

Impact

During liquidations, the PanopticMath::haircutPremia function will not remove the long premiums from liquidatee collateral shares to cover protocol losses and will instead mint him new collateral shares, resulting in a loss of funds for the protocol and the PLP.

Tools Used

Manual review, VS Code

Recommended Mitigation

When calling CollateralTracker::exercise in PanopticMath::haircutPremia, the provided realizedPremia parameter value should be negative to remove collateral shares from the liquidatee balance as follows:

function haircutPremia(
    address liquidatee,
    TokenId[] memory positionIdList,
    LeftRightSigned[4][] memory premiasByLeg,
    LeftRightSigned collateralRemaining,
    CollateralTracker collateral0,
    CollateralTracker collateral1,
    uint160 sqrtPriceX96Final,
    mapping(bytes32 chunkKey => LeftRightUnsigned settledTokens) storage settledTokens
) external returns (int256, int256) {
    unchecked {
        // get the amount of premium paid by the liquidatee
        LeftRightSigned longPremium;
        for (uint256 i = 0; i < positionIdList.length; ++i) {
            TokenId tokenId = positionIdList[i];
            uint256 numLegs = tokenId.countLegs();
            for (uint256 leg = 0; leg < numLegs; ++leg) {
                if (tokenId.isLong(leg) == 1) {
                    longPremium = longPremium.sub(premiasByLeg[i][leg]);
                }
            }
        }
        // Ignore any surplus collateral - the liquidatee is either solvent or it converts to <1 unit of the other token
        int256 collateralDelta0 = -Math.min(collateralRemaining.rightSlot(), 0);
        int256 collateralDelta1 = -Math.min(collateralRemaining.leftSlot(), 0);
        int256 haircut0;
        int256 haircut1;
        // if the premium in the same token is not enough to cover the loss and there is a surplus of the other token,
        // the liquidator will provide the tokens (reflected in the bonus amount) & receive compensation in the other token
        if (
            longPremium.rightSlot() < collateralDelta0 &&
            longPremium.leftSlot() > collateralDelta1
        ) {
            int256 protocolLoss1 = collateralDelta1;
            (collateralDelta0, collateralDelta1) = (
                -Math.min(
                    collateralDelta0 - longPremium.rightSlot(),
                    PanopticMath.convert1to0(
                        longPremium.leftSlot() - collateralDelta1,
                        sqrtPriceX96Final
                    )
                ),
                Math.min(
                    longPremium.leftSlot() - collateralDelta1,
                    PanopticMath.convert0to1(
                        collateralDelta0 - longPremium.rightSlot(),
                        sqrtPriceX96Final
                    )
                )
            );

            haircut0 = longPremium.rightSlot();
            haircut1 = protocolLoss1 + collateralDelta1;
        } else if (
            longPremium.leftSlot() < collateralDelta1 &&
            longPremium.rightSlot() > collateralDelta0
        ) {
            int256 protocolLoss0 = collateralDelta0;
            (collateralDelta0, collateralDelta1) = (
                Math.min(
                    longPremium.rightSlot() - collateralDelta0,
                    PanopticMath.convert1to0(
                        collateralDelta1 - longPremium.leftSlot(),
                        sqrtPriceX96Final
                    )
                ),
                -Math.min(
                    collateralDelta1 - longPremium.leftSlot(),
                    PanopticMath.convert0to1(
                        longPremium.rightSlot() - collateralDelta0,
                        sqrtPriceX96Final
                    )
                )
            );

            haircut0 = collateralDelta0 + protocolLoss0;
            haircut1 = longPremium.leftSlot();
        } else {
            // for each token, haircut until the protocol loss is mitigated or the premium paid is exhausted
            haircut0 = Math.min(collateralDelta0, longPremium.rightSlot());
            haircut1 = Math.min(collateralDelta1, longPremium.leftSlot());

            collateralDelta0 = 0;
            collateralDelta1 = 0;
        }

        {
            //@audit haircut0 & haircut1 must be transformed into negative values
            address _liquidatee = liquidatee;
--          if (haircut0 != 0) collateral0.exercise(_liquidatee, 0, 0, 0, int128(haircut0));
--          if (haircut1 != 0) collateral1.exercise(_liquidatee, 0, 0, 0, int128(haircut1));
++          if (haircut0 != 0) collateral0.exercise(_liquidatee, 0, 0, 0, int128(-haircut0));
++          if (haircut1 != 0) collateral1.exercise(_liquidatee, 0, 0, 0, int128(-haircut1));
        }

        ...
    }
}

Assessed type

Context

@c4-bot-6 c4-bot-6 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Apr 22, 2024
c4-bot-8 added a commit that referenced this issue Apr 22, 2024
@c4-bot-11 c4-bot-11 added the 🤖_242_group AI based duplicate group recommendation label Apr 22, 2024
@c4-judge c4-judge added the primary issue Highest quality submission among a set of duplicates label Apr 23, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as primary issue

@dyedm1
Copy link

dyedm1 commented Apr 26, 2024

Actually, the mechanism for protocol loss is executed when revoking delegated shares from the liquidatee to the liquidator. If there is protocol loss, the liquidatee will not have enough tokens to pay back the liquidator's original value + bonus, so shares are minted to the liquidator instead (getting them their payment, but lowering the share price). So, if we wanted to lower the liquidatee's protocol loss by revoking premium they paid, we would simply mint shares to their account (which would cover more of the revoked tokens to the liquidator, requiring less excess shares to be minted = lower protocol loss)

@dyedm1 dyedm1 added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Apr 26, 2024
@Picodes
Copy link

Picodes commented May 6, 2024

Downgrading to QA following the sponsor's remark.

@c4-judge
Copy link
Contributor

c4-judge commented May 6, 2024

Picodes changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax grade-b and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels May 6, 2024
@c4-judge
Copy link
Contributor

c4-judge commented May 6, 2024

Picodes marked the issue as grade-b

@C4-Staff C4-Staff added the Q-09 label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b primary issue Highest quality submission among a set of duplicates Q-09 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_242_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

6 participants