Zaros Part 1

Summary

The function _fillOrderis used in settlement of marketOrderand offchainOrder. That function _fillOrderis checking if the marginbalanceof user is greater than requiredMaintenanceMarginBalanceor requiredInitialMarginBalancedepending on whether position is increasing or decreasing. However, due to using wrong _sizeDeltain markPrice calculation, markPrice will be invalid which can lead to liquidations. The function _fillOrdershould calculate full position size in mark price calculation during trade because the position size used in liquidation should be same as position size in which order settlement should be checked.

Vulnerability Details

The function _fillOrderpasses sizeDelta to the function getAccountMarginRequirementUsdAndUnrealizedPnlUsd. The function getAccountMarginRequirementUsdAndUnrealizedPnlUsdcalculates required maintenance margin and required initial margin.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L407-L414

The above snippet uses maintenanceMarginfor comparison when position is decreasingand old position sizeis not zero. So, in below explanation, we will assume that it's the case.

Let's look at the implementation of getAccountMarginRequirementUsdAndUnrealizedPnlUsd. During the order settlement, targetMarketIdwon't be 0. It calculates markPricebased on sizeDeltainstead of considering entire position size. Due to this, markPricecalculation during this will differ from markPricecalculation of liquidation. notionalValueand requiredMaintenanceMarginis calculated based on markPrice. Now, if markPricecalculated during orderSettlementis less than markPriceduring liquidation, requiredMaintenanceMarginduring orderSettlementwill be less than requiredMaintenanceMarginduring liquidation. Due to this, trade opened will be subjected to instant liquidation which is unfair to users.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/TradingAccount.sol#L210-L249

Let's understand above vulnerability with following example:
Assumptions:

• User has only one open position(this is not a restriction but just ease of calculation is this case)

• Index price(returned by chainlink) for the user position's asset is $1(ease of calculation)

Note: The accrued funding and other things won't affect margin balance much when the transactions of order settlement and liquidation are in same block or in adjacent blocks.

Example:

Position size of user = -10_000(short position)

current skew of perp market = 100_000

skew scale = 1000_000

User wants to open long position of size = 1000 in same perp market. As user has short position opened and user wants to open long position, ctx.isIncreasingis falseand ctx.oldPositionSizeX18.isZero()is false. So, it will use maintenanceMarginfor comparison.

markPricecalculation being used in orderSettlementwith only sizeDeltaand ignoring old position size:

• skew = 100_000

• new skew = 100_000 + 1000 = 101000

• new position size = -9_000

• requiredMaintenanceMarginRate = 5%

• markPrice = index_price * (1 + (skew + newSkew) / (2 * skewScale)) = 1 * (1 + (100_000 + 101_000)/ (2 * 1000_000) = 1.1005

• requiredMaintenanceMargin = abs(newPositionSize) * markPrice * requiredMaintenanceMarginRate = 9000 * 1.1005 * 0.05 = 495.22500

markPricecalculation being used in checkLiquidation with consideration of whole position size after the above trade settlement:

• skew = 101_000

• position size considered for liquidation = 9_000

• new skew due to closing of position size for liquidation = 101000 + 9000 = 110_000

• requiredMaintenanceMarginRate = 5%

• markPrice = index_price * (1 + (skew + newSkew) / (2 * skewScale)) = 1 * (1 + (101_000 + 110_000)/ (2 * 1000_000) = 1.1055

• requiredMaintenanceMargin = abs(newPositionSize) * markPrice * requiredMaintenanceMarginRate = 9000 * 1.1055 * 0.05 = 497.47500

As we can see from the above two instances, requiredMaintenanceMarginin liquidation is more than requiredMaintenanceMarginduring trade opening. So, the trade can be opened but subjected to liquidation just after settlement of trade.

Impact

The trades may succeed requiredMaintenanceMarginin order settlement which will lead of settlement of order. But it will lead to liquidation as soon as trade is opened.

Tools Used

Manual review

Recommendations

The function _fillOrdershould calculate full position size in mark price calculation during trade because the position size used in liquidation should be same as position size in which order settlement should be checked.