Zaros Part 1

Summary

When assessing account's health, the required margin does not include liquidation fees. As a result, during liquidations there is insufficient margin to pay both marginCollateralRecipient and liquidationFeeRecipient.

Vulnerability Detail

During liquidation, an account is assessed to be liquidatable if requiredMaintenanceMargin > marginBalance.

requireMaintenanceMargin is calculated based on a percentage of notionalValue, so if price falls to a certain level, the account is marked as liquidatable and the maintenance margin is paid out to the marginCollateralRecipient.

However, the issue lies with a liquidation fee which must also be paid out to liquidationFeeRecipient on top of the maintenance margin:

This liquidation fee should always be added to requiredMaintenanceMargin to ensure sufficient collateral to pay both marginCollateralRecipient and liquidationFeeRecipient.

Impact

In deductAccountMargin, settlementFees are paid before pnlUsd. So liquidationFeeRecipient will receive the full amount but marginCollateralRecipient will not.

marginCollateralRecipientwould likely be the protocol or external LPs, who are the counterparty to traders. If traders earn profit, marginCollateralRecipient lose money. But in this scenario where traders are liquidated, marginCollateralRecipient does not earn the full amount and is therefore receiving less for the risk they are taking on.

Code Snippet

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/TradingAccount.sol#L210
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol#L152

Tool used

Manual Review

Recommendation

When calculating required margin in getAccountMarginRequirementUsdAndUnrealizedPnlUsd, add the potential liquidation fee that must be paid out if the account is liquidated. This would also prevent settling orders that just meet the maintenance margin, but cannot cover any potential liquidation fees if the account were to be liquidated after.