Zaros Part 1

Lines of code

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/OrderBranch.sol#L134-L136

Impact

When checking if a user is liquidatable in simulateTrade, the wrong RequiredMaintenanceMarginUsd is used. This can prevents users to improve the health of their account and lead to unnecessary liquidations down the road.

Proof of Concept

A user can open a marketOrder by calling createMarketOrder. Before recording the marketOrder of the user, the trade is simulated to ensure that specific margin requirements, the initialMarginRequirement when increasing a position and maintenanceMarginRequirement when reducing a position, are met.

First the requiredInitialMargin, requiredMaintenanceMargin and the accounts totalUnrealizedPnl after the trade are determined:

Second the accounts marginBalance after the trade is calculated:

Third, the maintenanceMarginRequirement before the trade is calculated:

And last, it is checked if the account is liquidatable after the trade:

The issue arises from the fact that, when checking if the user will be liquidatable after the trade, the maintenanceMarginRequirement before the trade is compared to the marginBalance after the trade. This does not make sense since, to check if a use is liquidatable after the trade, the margin requirements after the trade should be used. Otherwise, users might be prevented to create a marketOrder which would increase the health of their accounts and lead to preventable liquidations down the road.

Example

User opens position:
• user deposits 350 USDC as collateral, the loanToValue value for USDC is 1 so the marginBalance of the account is therefore 350 USD
• user opens a SHORT position on ETH. The market price of ETH is 3500 USD, the initalMarginRate is 10% => initialMarginRequirement = 3500 USD * 10% = 350 USD
Marketprice goes in wrong direction and user gets close to being liquidatable:
• market price of ETH moves to 3660 USD => P&L for the position is 3500 – 3670 = -170 USD
• maintenance margin rate is 5% so the requires maintenanceMargin is 3670 * 5% = 183,5 USD
• margin + PNL of the account is 350 – 170 = 180 USD => account is liquidatable

If the user would be able to closes half of his position, he would increase the health of this account and thereby the overall health of the protocol: (without fees for simplicity)
• user closes half of his position (0,5 ETH) and pays the loss of 170 USD from his collateral
• user collateral = 350 – 170 = 180 USD
• maintenance margin now is (3670 * 0,5 ETH) * 5% = 91,75 USD way below the collateral value of the user => user is no longer liquidatable

But since he is prevented from trading he will be liquidated, all his positions will be closed and he will need to pay the liquidation fee.

Recommended Mitigation Steps

To determine if an account will be liquidatable after the trade, make sure to compare the maintenanceMarginRequirments of the account after the trade with the marginBalance of the account after the trade. This will ensure that an account can always open a new marketOrder that increases the health of the account and thereby the health of the protocol.