Zaros Part 1

Impact

The bug described here, exploits precision loss in fixed-point arithmetic calculations during the liquidation process. It allows a blach hat to exploit position sizes, and also prices, to create scenarios where accounts that should be liquidated, are not. And even where liquidations result in significantly less collateral being recovered, than should be. More of the possible impact is as follows =

1. Accumulation of bad debt in the system as undercollateralized positions still remain open.

2. Unfair liquidations where users lose more collateral than they should lose.

3. A black hat could profit by creating positions that exploit these precision errors.

4. Systemic insolvency, as the protocol fails to maintain the accurate collateralization ratios.

Code snippet of bugs

The bug stems from the interaction between LiquidationBranch.sol and PerpMarket.sol:

In [LiquidationBranch.sol](https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol):

In [PerpMarket.sol](https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/PerpMarket.sol):

POC using foundry

Attack vector =

1. Alice creates a large position with 1000 units of collateral. Bob creates a smaller position with 10 units of collateral.

2. Black Hat identifies the precision loss bug. Black Hat then creates multiple small positions, each just above the liquidation threshold.

3. These price movements should trigger liquidations, due to precision loss, some of Black Hat's positions remain open.

4. Alice's large position is incorrectly liquidated, recovering less collateral than it should. Bob's smaller position is not liquidated when it should be, remaining as bad debt in the system.

5. Black Hat repeats this process again, accumulating small positions that should be liquidated but aren't.

6. Over time, this leads to a significant accumulation of bad debt and unfair liquidations.

Bug lies in the fixed-point arithmetic used in the getMarkPrice function. The multiple divisions and conversions between SD59x18 and UD60x18 can lead to precision loss, especially for very small or very large numbers.

Look at this calculation of priceImpactBeforeDelta:

If skew is very small compared to skewScale, this division results in 0 due to rounding, even when there should be a small price impact.

In the final calculation:

This division by 2 could enable rounding errors, especially if priceBeforeDelta and priceAfterDelta are very close but not actually identical.

These precision losses compound up, when used in the liquidation calculations, leading to scenarios where:

1. Calculated markPrice is slightly off, causing positions to remain open when they should be liquidated.

2. positionUnrealizedPnl calculation in the liquidation process is not correct, leading to incorrect liquidation amounts.

The impact is of course magnified, for very large or very small positions, and in markets with high skew or rapidly changing prices.

Recommended Mitigation Steps

1. To prevent this completely, use a minimum price impact to prevent divisions resulting in zero:

   SD59x18 priceImpactBeforeDelta = max(skew.div(skewScale), MINIMUM_PRICE_IMPACT);

2. Use higher precision intermediates in calculations:

   UD60x18 markPrice = priceBeforeDelta.add(priceAfterDelta).mul(SCALE).div(ud60x18Convert(2)).div(SCALE);

3. Use extra sanity checks on liquidation results:

   require(recoveredCollateral >= minimumExpectedCollateral, "Liquidation recovered too little collateral");