Zaros Part 1

Lines of code

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

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/PerpMarket.sol#L98-L122

Impact

When filling a position, the calculation of the margin requirements uses the wrong price. This leads to an over- / underestimation of the required margin and can, beside other things, result in the opening of a position which has a buffer to liquidation that is lower than intended and will thereby be become liquidatable earlier than expected. In the worst case scenario, a position which was just opened is liquidatable right away.

Proof of Concept

A user needs to satisfy the initialMargin requirements to be able to open a position in a market. These requirements are calculated in the function getAccountMarginRequirementUsdAndUnrealizedPnlUsd. In this function:

• the current markPrice is calculated using an oracle price and the change in position size

• this markPrice is multiplied by the size of the position to get the notionalValue of the position

• the notionalValue is used to calculate the required initialMargin

The calculation of the notinalValue uses the notinalValue which in turn is calculated by using the markPrice. The issue arises from the fact that the markPrice is the average between the price before the position is filled and after the position is filled:

This results in an over- or underestimation of the required margin which puts the position closer to liquidation than intended. This is because, when calculating if a user is liquidatable or not, the price after the trade is used. In extreme cases where the positions are big and the difference between initial margin and maintenance margin is small, an open Position can be liquidatable right after it is created.

The following example shows that every position meets the initalMargin when it is opened, without any further change, it does not meet it any more when checking for liquidation:

Example: (no fees and no decimals for simplicity)

User has no positions, the skew of the market he is interacting with is 0.
Values of the market:
- Scew: 0
- scewScale: 10,000,000

Global values:
- initalMarginRequirements: 1%
- mantenanceMarginRequirements: 0,5%

User values:
- Position size: 1 ETH
- Collateral value: 35,88 USD

Calculation of the initialMargin when opening the position:

MarkPrice = 3,587,50 USD/ETH

notinalValue = 3,587.50 USD/ETH * 1 ETH = 3,587.50 USD
initialMargin = 3,587.50 USD * 1% = 35,88 USD
maintenaceMargin = 3,587.50 USD * 0,5% = 17,94 USD

User satisfies the initialMargin of 35,88 USD because his collateral is worth 35,88 USD and the position is created.

Without changing anything, if checkLiquidatableAccounts is called, the basis for liquidating accounts, the account does not meet the requirements for the initial margin anymore and the required maintenance margin is higher than when the position is created. That is becasue there is no skewDelta and therefore priceBeforeDelta and priceAfterDelta are the same, resulting in a markPrice equal to the priceAfterDelta in the first calculation and therefore the marginRequirements become higher:

skewScale = 10,000,000;
skew = 500,000

priceImpactBefore = 500,000 / 10,000,000 = 5%

priceBeforeDelta = priceAfterDelta = 3500 USD/ETH + (3500 USD/ETH * 5%) = 3675,00 USD/ETH
notinalValue = 3675,00 USD/ETH * 1 ETH = 3675,00 USD

initialMargin = 3,675.00 USD * 1% = 36,75 USD (value before: 35,88 USD)
maintenaceMargin = 3,675.00 USD * 0,5% = 18,38 USD (value before: 17,94 USD)

If the values for skewScale, skew, skewDelta, initalMarginRequirements and mantenanceMarginRequirements are set in an unfortunate combination, this can even lead to positions which can be liquidated right after they are created.

Recommended Mitigation Steps

When calculating the margin requirements for filling a position, make sure to use the same price as for calculating if a user can be liquidated. This would be the priceAfterDelta in the formulars above.