Consecutive symbol price updates can be exploited to drain protocol funds
Repeatedly updating the symbol prices for the symbols used in Party A's positions mid-way through a liquidation while maintaining the same Party A's UPnL and total unrealized losses leads to more profits for Party B and effectively steals funds from the protocol.\\nThe `setSymbolsPrice` function in the `LiquidationFacetImpl` library is used to set the prices of symbols for Party A's positions. It is called by the liquidator, who supplies the `PriceSig memory priceSig` argument, which contains, among other values, the prices of the symbols as well as the `upnl` and `totalUnrealizedLoss` of Party A's positions.\\nParty A's `upnl` and `totalUnrealizedLoss` values are stored in Party A's liquidation details and enforced to remain the same for consecutive calls to `setSymbolsPrice` via the `require` statement in lines 90-95.\\nHowever, as long as those two values remain the same, the liquidator can set the prices of the symbols to the current market prices (fetched by the Muon app). If a liquidator liquidates Party A's open positions in multiple calls to `liquidatePositionsPartyA` and updates symbol prices in between, Party B potentially receives more profits than they should have.\\nThe git diff below contains a test case to demonstrate the following scenario:\\nGiven the following symbols:\\n`BTCUSDT`\\n`AAVEUSDT`\\nFor simplicity, we assume trading fees are 0.\\nParty A's allocated balance: `100e18 USDT`\\nParty A has two open positions with Party B:\\nID Symbol Order Type Position Type Quantity Price Total Value CVA LF MM Total Locked Leverage\\n1 BTCUSDT LIMIT LONG 100e18 1e18 100e18 25e18 25e18 0 50e18 2\\n2 AAVEUSDT LIMIT LONG 100e18 1e18 100e18 25e18 25e18 0 50e18 2\\nParty A's available balance: 100e18 - 100e18 = 0 USDT\\nNow, the price of `BTCUSDT` drops by 40% to `0.6e18 USDT`. Party A's `upnl` and `totalUnrealizedLoss` are now `-40e18 USDT` and `-40e18 USDT`, respectively.\\nParty A is insolvent and gets liquidated.\\nThe liquidator calls `setSymbolsPrice` for both symbols, setting the price of `BTCUSDT` to `0.6e18 USDT` and the price of `AAVEUSDT` to `1e18 USDT`. The `liquidationDetails` of Party A are as follows:\\nliquidationType: `LiquidationType.NORMAL`\\nupnl: `-40e18 USDT`\\ntotalUnrealizedLoss: `-40e18 USDT`\\ndeficit: 0\\nliquidationFee: `50e18 - 40e18 = 10e18 USDT`\\nThe liquidator first liquidates position 1 -> Party B receives `40e18 USDT` + `25e18 USDT` (CVA) = `65e18 USDT`\\nNow, due to a volatile market, the price of `AAVEUSDT` drops by 40% to `0.6e18 USDT`. The liquidator calls `setSymbolsPrice` again, setting the price of `AAVEUSDT` to `0.6e18 USDT`. `upnl` and `totalUnrealizedLoss` remain the same. Thus the symbol prices can be updated.\\nThe liquidator liquidates position 2 -> Party B receives `40e18 USDT` + `25e18 USDT` (CVA) = `65e18 USDT`\\nParty B received in total `65e18 + 65e18 = 130e18 USDT`, which is `30e18` USDT more than Party A's initially locked balances. Those funds are effectively stolen from the protocol and bad debt.\\nConversely, if both positions had been liquidated in the first call without updating the symbol prices in between, Party B would have received `40e18 + 25e18 = 65e18 USDT`, which Party A's locked balances covered.\\n\\nHow to run this test case:\\nSave git diff to a file named `exploit-liquidation.patch` and run with\\n```\\ngit apply exploit-liquidation.patch\\nnpx hardhat test\\n```\\n
Consider preventing the liquidator from updating symbol prices mid-way of a liquidation process.\\nOr, alternatively, store the number of Party A's open positions in the `liquidationDetails` and only allow updating the symbol prices if the current number of open positions is still the same, effectively preventing the liquidator from updating the symbol prices once a position has been liquidated.
A malicious liquidator can cooperate with Party B and by exploiting this issue during a volatile market, can cause Party B to receive more funds (profits, due to being the counterparty to Party A which faces losses) than it should and steal funds from the protocol.
```\\ngit apply exploit-liquidation.patch\\nnpx hardhat test\\n```\\n