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

Option Sellers can DoS Option Buyers using EOA Accounts from exercising options that are ITM. #274

Open
c4-bot-8 opened this issue Apr 20, 2024 · 3 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue edited-by-warden grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_274_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@c4-bot-8
Copy link
Contributor

c4-bot-8 commented Apr 20, 2024

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L773-L782
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L455-L456

Vulnerability details

Impact

Option Buyers could not be able to exercise their options when they are ITM, which could end up the Option Buyers not been able to make a profit on their trading strategies.

Proof of Concept

When long Options are ITM, the Option Buyers are earning a profit, as opposed to Option Sellers who are losing money. So, Option Buyers have the incentive to close/burn/exercise their long options and walk away with their profits, while, Option Sellers could be incentivized (if there might be a way) to try to DoS Option Buyers from closing their long position while they are ITM.

When burning/closing a long option that is ITM, the SFPM determines the amount of tokens that needs to be swapped in order for the position to be closed. When a PanopticPool is closing an ITM option, the payer to the UniV3Pool for doing the swap is the PanopticPool itself (those swapped tokens are later charged to the user), this means, when burning a long ITM option, the SFPM will send funds from the PanopticPool to the UniV3Pool. But this means that the PanopticPool needs to have those tokens on its balance, else, the swap will revert because the payer (PanopticPool) doesn't have enough funds.

Full Execution Flow when burning long options ITM

function _validateAndForwardToAMM(
    ...
) internal returns (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalMoved) {

    ...

    // calls a function that loops through each leg of tokenId and mints/burns liquidity in Uni v3 pool
    (totalMoved, collectedByLeg, itmAmounts) = _createPositionInAMM(
        univ3pool,
        tokenId,
        positionSize,
        isBurn
    );

		//@audit => Checks if position is ITM
		
    if (tickLimitLow > tickLimitHigh) {
        // 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
        if ((LeftRightSigned.unwrap(itmAmounts) != 0)) {
            totalMoved = swapInAMM(univ3pool, itmAmounts).add(totalMoved);
        }

        (tickLimitLow, tickLimitHigh) = (tickLimitHigh, tickLimitLow);
    }

    ...
}

=> swapInAMM(...)if position is ITM

  ⇒ _univ3pool.swap(...)Does a swap on the UniV3Pool

function swapInAMM(
    IUniswapV3Pool univ3pool,
    LeftRightSigned itmAmounts
) internal returns (LeftRightSigned totalSwapped) {
    ...

        ...

        // construct the swap callback struct
            data = abi.encode(
                CallbackLib.CallbackData({
                    ...

                    //@audit-info => The swap's payer is the caller. When closing options, the caller is a PanopticPool!
                    payer: msg.sender
                })
            );

        ...

        (int256 swap0, int256 swap1) = _univ3pool.swap(
            ...
            data
        );

        //@audit-info => The payer of the swap was the same caller of this contract, when OptionBuyers are closing ther positions, caller is a PanopticPool!

        //@audit-info => When caller is a PanopticPool, the token paid by the caller because of the swap is taken from the PanopticPool
        // Add amounts swapped to totalSwapped variable
        totalSwapped = LeftRightSigned.wrap(0).toRightSlot(swap0.toInt128()).toLeftSlot(
            swap1.toInt128()
        );
}

uniswapV3SwapCallback(...)← SFPM receives the callback from Uniswap and pays the amount0 and amount1 of tokens specified by the UniV3Pool. The payer is the PanopticPool who called the SFPM.burnTokenizedPosition(...)

function uniswapV3SwapCallback(
    ...
) external {
    
    ...

    //@audit-info => The payer that was encoded on the `swapInAMM()` will pay the required amount of tokens to the UniV3Pool for the swap
    //@audit-issue => If the caller (PanopticPool) doesn't have enough tokens, the closing of the option will be reverted!

    // Pay the required token from the payer to the caller of this contract
    SafeTransferLib.safeTransferFrom(token, decoded.payer, msg.sender, amountToPay);
}

⇒ If the swap reverts, the whole tx will revert, causing the OptionBuyer to not be able to exercise his position that is ITM.

Option Sellers who are not willing to accept a loss can frontrun Option Buyers before their long ITM options are burnt, the Option Sellers would mint short options to drain the available liquidity on the PanopticPool and move it to the UniV3Pool, this would cause that the PanopticPool runs out of available funds, because all the funds were moved to the UniV3Pool due to the new short options.

This would cause that when the OptionBuyer tries to close his long option ITM to fail, the reason is because the PanopticPool would have not enough balance to pay for the required tokens to do the swap.

  • Even if the OptionBuyer would deposit more tokens on the CollateralTrackers (so that the PanopticPool receives more funds), the Option Sellers can continue to frontrun and open new short options to keep moving the liquidity out of the PanopticPool to the UniV3Pool.
    • The OptionBuyer would deposit more tokens on the Collateral Tracker which would be transfered onto the PanopticPool! (tx1)
    • The Option Seller would immediately mint new shorts to move the new deposits out of the PanopticPool to the UniV3Pool! (tx2)
    • The OptionBuyer tx to close his ITM long option would be executed and reverted because the PanopticPool has not enough tokens for the swap (because OptionSeller opened a short position that moved the funds from the PanopticPool to the UniV3Pool!). (tx3)

Option Buyers can't forceExercise the new short positions because the forceExercise() only works for positions with a long, but if the new positions contains only shorts, then, those positions can't be forceExercised.

When minting shorts, it is not enforced that shorts can't deploy all the available liquidity on the PanopticPool to the UniV3Pool (only when minting longs there is a check to validate the spread between liquidity on the PanopticPool and the UniV3Pool)

Tools Used

Manual Audit

Recommended Mitigation Steps

I think there a couple of ways to mitigate this problem and don't leave any possibility to OptionSellers from forcefully DoSing the Option Buyers from closing their ITM long options.

  1. Implement a liquiditySpread check when minting shorts and enforce that a minimum amount of liquidity must be available on the PanopticPool at all times. In this way, the Option Sellers can't move all the liquidity to the UniV3Pool by continously creating new shorts

  2. Create some bundlers that allows the Option Buyers using EOA accounts to deposit tokens to the CollateralTrackers and in the same tx they can also burn their ITM long options, in this way, the OptionSellers can't even touch the new liquidity added specifically to cover for the swap while closing the ITM long option.

Assessed type

Context

@c4-bot-8 c4-bot-8 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Apr 20, 2024
c4-bot-5 added a commit that referenced this issue Apr 20, 2024
@c4-bot-12 c4-bot-12 added the 🤖_274_group AI based duplicate group recommendation label Apr 22, 2024
@dyedm1 dyedm1 added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Apr 26, 2024
@dyedm1
Copy link

dyedm1 commented Apr 26, 2024

Option Sellers could be incentivized (if there might be a way) to try to DoS Option Buyers from closing their long position while they are ITM.

There is no such incentive -- option buyers have no effect on seller p&l besides the premium they pay. Sellers are simply lending an asset (LP position) to buyers. If anything they have the opposite incentive -- they get premium and liquidity when buyers close their positions.

The DoS route described is quite expensive and can be bypassed in multiple ways. Also, option buyers don't use forceExercise to close their own positions.

@Picodes
Copy link

Picodes commented May 1, 2024

Downgrading to QA as I fail to see the incentive to do this as well

@c4-judge c4-judge removed the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label May 1, 2024
@c4-judge
Copy link
Contributor

c4-judge commented May 1, 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 labels May 1, 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 edited-by-warden grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_274_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

7 participants