code-423n4 / 2023-11-panoptic-findings

0 stars 0 forks source link

Use of Uniswap premia (fees generated) to calculate the cost of options is open to manipulation by performing swaps in Uniswap, which can lead to artificial losses for options buyers. They also won't know the price of the option ahead of time so can't protect themselves from it. #496

Closed c4-bot-5 closed 9 months ago

c4-bot-5 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L206 https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1073

Vulnerability details

Impact

The Panoptic documentation says "The key difference between the pricing of regular options in TradFi and in Panoptic is the way the premium is calculated. Instead of requiring users to pay for their options upfront, the pricing of a Panoptic option is path-dependent and will grow at each block as long as the spot price is within range of the option strike price." The options are priced based on the amount of fees generated in Uniswap for the applicable liquidity. As the documentation explains, "providing concentrated liquidity in Uniswap v3 generates a payoff [fees generated] that is mathematically identical to selling a put option".

That means that malicious users can manipulate the cost of an option (as further described below) and force options buyers to pay an inflated price for their option, and, because the option buyer has to enter the trade without knowing how much the option will cost, the option buyer won't have much recourse. Even though the malicious user has to pay the fees on the swap, it is possible for them to come out ahead by selling lots of positions within the same liquidity range, so that they only have to pay the fees for one swap but they drive up the amount they receive on many positions.

Proof of Concept

A person could sell put options in Panoptic and then turn around and make lots of swaps in Uniswap within that liquidity range to drive up the price that would be paid by the buyer of the option, possibly aided by flash loans. Even though he must pay the fees on these swaps, you have to keep in mind that a spread is added to the fees (see below) but, also, to make it profitable for himself, he could sell a lot of options within the same liquidity range so that every time he makes a swap, he drives up the amount he will receive on lots of positions (but still only has to pay fees for one swap).

In addition to tracking, we also want to track those fees plus a small spread. Specifically,
        we want:

              gross_feesCollectedX128 = net_feesCollectedX128 + owed_feesCollectedX128

       where 

              owed_feesCollectedX128 = feeGrowthX128 * R * (1 + spread)  

Tools Used

Manual review

Recommended Mitigation Steps

You could consider blacklisting accounts that have a history of doing this or possibly not counting any fees that result from trades executed by accounts that have sold an open put option on Panoptic, but both of these are easy to get around by just using a different address. Or maybe, before closing an option, you could check whether the fees generated while it has been open are within a margin of the fees generated by it, on average, over the course of the past year, and, if they are not, reduce the cost down until it is within that margin. But, honestly, I don't know if it is worth all the extra complexity. It really depends on how much this behavior occurs.

It is definitely something at least that options buyers must be aware of, though. It is incumbent on them to monitor their positions and notice if something like this is taking place, but the truth is that someone could do this very quickly if they wanted to.

Assessed type

Other

c4-judge commented 10 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 10 months ago

dyedm1 (sponsor) disputed

dyedm1 commented 10 months ago

Yes, fees as premium is the core component of pricing in Panoptic. Certainly the fees can be "manipulated", but you could say the same of IV in traditional options. Ultimately fee manipulation isn't profitable unless the buyers paying that fee hold a really large portion of the entire active liquidity in a pool purchased directly from you (the fees and the oportunity cost that comes from those fees is real, they are actually paid out to people so the more premium has to be going to you from the buyer than fees paid to other LPs). This certainly won't be a common occurrence on most liquid pools, and the risk factors for this will be clear for buyers and frontends to see on less liquid pools. Ultimately, this is a key design decision in our protocol that is generally resistant to profitable manipulation, but can occasionally be manipulated somewhat in certain low-liquidity scenarios as every financial instrument can.

Also important to note that there is no options buying or selling in the SFPM and you can only manage your own liquidity. These types of situations have no effect on the SFPM - just options protocols built on it that use the Panoptic formula for options premium available in the SFPM.

Picodes commented 9 months ago

Was this documented before the contest?

To me, this fulfills the definition of Med severity under C4's rules "leak value with a hypothetical attack path with stated assumptions, but external requirements" and is typically the kind of issue that will be in the report as "sponsor acknowledged" - the leak of value being in the option protocol using the SFPM

c4-judge commented 9 months ago

Picodes marked the issue as satisfactory

c4-judge commented 9 months ago

Picodes marked the issue as selected for report

dyedm1 commented 9 months ago

Was this documented before the contest?

Yes, our whitepaper and documentation clearly states we use Uniswap fees as our core pricing mechanism.

To me, this is a natural part of the contract option buyers enter into with sellers.

Imagine a hypothetical binary options protocol that uses Chainlink oracles (or some other external pricing mechanism that can be considered infallible for the purposes of an audit). Alice and Bob deposit 10 million USDC each to take opposite sides of an bet with S = 2000 USD/ETH that settles at block number N. If the real price of ETH according to our robust pricing mechanism is < 2,000 USDC, Alice receives 20 million USDC. Otherwise, Bob receives the 20 million.

Suppose that 5 minutes before the final price update, the average price of ETH is 1,999 USD. Bob wants the 20 million, so he puts considerable buy pressure on ETH over the next 5 minutes, raising the price over 2,000. At the time the final price measurement is taken by the oracle, the actual price of ETH is 2,005 USD across all markets and Bob wins the bet.

Alice may be disappointed at this, but is this an issue with the smart contract? After all, the price of ETH was in reality over 2,000, so the contract she entered into executed as expected. If this hypothetical contract was audited by C4, the situation described above might be included in an analysis report or something, but definitely wouldn't be a medium-severity issue.

The same principle applies here. Uniswap fees are part of the agreed pricing mechanism between buyers and sellers, so whether they are manipulated or otherwise "unfair" does not matter. The only important thing is if the contract executes according to the stated terms (which it does in this situation).

Picodes commented 9 months ago

Yes, our whitepaper and documentation clearly states we use Uniswap fees as our core pricing mechanism.

Sure, my question was more about documentation regarding the fact that it may be profitable to artificially inflate fees to make it look like the "implied volatility" is higher than expected.

Your example is interesting because in my view it's fair to consider this a medium-severity issue if it is not documented and disclosed by the protocol. When buying an option, especially in a permissionless system you want users to have all the prerequisites in mind, and in the case you're describing users shouldn't enter options markets "if the size of the payoff isn't small compared to the size of manipulating the market price". If we don't consider this a potential issue I don't see why we would consider oracle manipulations in lending markets an issue, and this led to a lot of attacks.

In the SPFM case, because owed_feesCollectedX128 is approximated for liquidity that is not in the pool, there are conditions (for example when removed liquidity is significant compared to the pool size) where it's profitable for someone to artificially inflate the "implied volatility", and especially as operations involving the spread are not straightforward it's a fair issue to me.

Please let me know if I am missing something or if you still disagree!

dyedm1 commented 9 months ago

If we don't consider this a potential issue I don't see why we would consider oracle manipulations in lending markets an issue, and this led to a lot of attacks.

The difference between this (and my example) and oracle manipulation is that oracle manipulation usually involves exploiting a flaw in the oracle to report a price to a protocol that is unreflective of reality. In this case, an attacker manipulates the underlying market (not an oracle or protocol) to bring about conditions in the protocol reflective of the state of the market. If that were really a valid issue, then it would be included in reports for every protocol that considers market prices, because it's almost always technically possible to manipulate it profitably in some way (even by starting a fake ETF rumor lol)

In the SPFM case, because owed_feesCollectedX128 is approximated for liquidity that is not in the pool, there are conditions (for example when removed liquidity is significant compared to the pool size) where it's profitable for someone to artificially inflate the "implied volatility", and especially as operations involving the spread are not straightforward it's a fair issue to me.

The spread multiplier is actually pretty straightforward to compute from the liquidity utilization, there's even a Desmos linked in the code.

There are definitely conditions where manipulation could be profitable, just like in every protocol that relies on market data, but only for very illiquid pools with poor LP diversity.

It's also important to note that all these arguments about traders having enough data to make informed decisions are kind of superfluous, because none of these situations affect SFPM traders at all. The SFPM just exposes processed data about Uniswap fees that protocols can use any way they want.

It was mentioned that this specific risk should be documented so traders can choose pools that aren't vulnerable to, but that's in the scope of options protocols built on the SFPM and not the SFPM itself. For example, if a protocol wanted to mitigate this risk (assuming they sold options based on the data exposed in the SFPM), they could whitelist pools or simply not allow deployments on illiquid pools. We actually have a few specific measures to address this sort of risk in Panoptic, both in-protocol and in the frontend.

If we consider this part of the SFPM as a sort of oracle for fee data, none of the situations described seem out of the ordinary. You can have perfect oracles for all sorts of different information on different assets with no bugs that function as intended/documented, but some assets can be easily manipulated, causing downstream issues in protocols that use oracles for that asset. I don't think that points to an issue in the oracle phase of the supply chain - oracles are only responsible for providing correct data. Ultimately, it's the responsibility of protocols to select suitable assets.

IF this issue was to be raised, I think it would be in an audit for Panoptic (or other SFPM-based options protocols), not the SFPM. In the current scope, it only really makes sense as an analysis. These sorts of attacks/situations aren't usually (and really can't be) prevented at this (oracle) stage in the supply chain, so it doesn't seem productive to include this as an issue in the audit.

Picodes commented 9 months ago

Aside from the fact that I was mentioning oracle manipulations without flaws in the oracle themselves (for example the Mango market situation), I can agree with this reasoning and downgrade this issue based on the fact that this is mostly a risk for integrators and not users of the SFPM.

c4-judge commented 9 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

Picodes marked the issue as not selected for report