Significant divergence in unrealizedPnL calculation of Perp protocol vs Depository can lead to undercollateralization

Summary

During high volatility period (as experienced in last 4 trading sessions from 11 Jan to 15 Jan), unrealized P/L calculations in PerpDepository contract cannot keep up with the unrealized P/L changes in the Perp protocol. While the former uses a simplistic formula involving time-weighted average price, latter is actual reflection of liquidity v/s taker demand. rebalance and rebalanceLite functions for negative P/L use the unrealizedPnL function of Perp protocol to limit the amount of rebalancing. When there is a large divergence of unrealized PnL on Perp protocol, auto rebalancing does not allow for full rebalancing. In the extreme case, this can lead to margin breach & asset (WETH) liquidation

Vulnerability Detail

Unrealized P/L calculation in Depository uses redeemable UXD, current clearingHouse account balance & 15 block TWAP in getUnrealizedPnL function in PerpDepository contract. Perp curie contracts, on the other hand, calculate unrealized P/L using the getPnlAndPendingFee function in AccountBalance contract.

I have deployed the Depository on optimism goerli (0x3d5573e7f3a942bc8b508c3adb69acfb73f81835) on Jan 10'th - total position size of $40.86 UXD. Between 10'th and 15'th, $ETH rose by ~25%. Volatility & Open Interest has increased significantly in 5 days.

Unrealized P/L on Perp protocol is significantly higher than that calculated by depository contract (more than double).

Etherscan screenshot shows unrealized PnL on the UXD protocol.. (1.013)

Etherscan screenshot shows unrealized PnL on the Perp protocol.. (2.989)

Impact

Proof of concept (all numbers are representation only)

insurance fund is fully utilised
Perp protocol margin is 1.5 million
unrealized P/L on UXD protocol is -1 million
unrealized P/L on Perp protocol is -1.8 million
Bob calls _rebalanceNegativePnlWithSwap function to the extent of -1 million (that's the limit that Bob can rebalance at the time)
$ETH price increases further -> and unrealized P/L increases from -0.8 million to -1.5 million
underlying WETH gets liquidated and $UXD becomes undercollateralized

In edge cases, the basis risk between both P/L computation methodologies can cause asset liquidation and under collateralization. Since this is a rare occurrence in specific market conditions, I've marked this risk asMedium.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L430

https://github.com/perpetual-protocol/perp-curie-contract/blob/27ea8e2c4be37d1dd58c1eed3b3cc269d398a091/contracts/AccountBalance.sol#L251

Tool used

Manual Review

Recommendation

On the negative rebalancing side, primary objective is to protect Perp position from Liquidiation. Recommend using getUnrealizedPnL of Perp protocol over the TWAP based unrealizedPnL computed by depository contract to regulate rebalancing amount. Most of the times, both numbers are close to each other - so it won't make much difference as to which methodology is used in calmer times

But, in the event of extremely high volatility where spot price can significantly be out-of-line compared to Perp price, funding rate might not be able to quickly stabilize the market - in such cases, unrealizedPnL of Perp protocol is what matters as the Perp protocol is adjusting to real-time supply-demand.

Duplicate of #249

sherlock-audit / 2023-01-uxd-judging

0Kage - Significant divergence in unrealizedPnL calculation of Perp protocol vs Depository can lead to undercollateralization #409