sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

WATCHPUG - `Liquidation` should not put the market into a worse state (more bad debt). #140

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

WATCHPUG

high

Liquidation should not put the market into a worse state (more bad debt).

Summary

Vulnerability Detail

LiquidationFee should not exceed the total collateral of the position because that would mean that the market is paying for the liquidation with system-wide bad debt and putting the market in a worse state.

In the current implementation, liquidationFee is a percentage based on the size of the position (bounded by minLiquidationFee and maxLiquidationFee). In normal circumstances, this should work without causing any trouble. However, when the time comes that the price suddenly drops ("flash crash") or a special occurrence happens where the entire blockchain experiences downtime (happened to OP, Polygon, Arbitrum, and BSC), resulting in an inevitable sudden price change, liquidations that result in shortfalls will become possible and result in an increase of system bad debt.

PoC

Given:

There are only two users, one with a long position and another one with a short position:

When:

  1. The price suddenly rises to 18
  2. PNL to long: 10 * 10 == 100, PNL to short: -100
  3. While the remaining collateral of the short user is 0, liquidation will take a fee and incur a bad debt which will result in the long user being unable to redeem all the profits.

Impact

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/Position.sol#L292-L322

Tool used

Manual Review

Recommendation

Based on the fact that the liquidation will most certainly be executed by the bots and for the bots, as long as the return to execute a transaction is greater than the gas cost, they would do it without complaining about the low margin.

A better design is to ask for a fixed amount of liquidation deposit denominated in the native token when opening the account for the first time (similar to GMX, MakerDAO, Liquity), and combine it with a portion of the remaining collateral, but never go beyond and cause any bad debt.

Duplicate of #72

sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

d

panprog commented:

medium or low because it's a protocol design choice and during high volatility gas prices and uncertaincy can be very high (lots of transactions can revert) so that liquidators need bigger incentive to keep liquidating.