Zaros Part 1

Summary

The protocol implements a liquidation priority list with their respective LTV ratio, this list also defines the types of collateral that can be deposited which prevents other collateral from being deposited and this list is also used when liquidations take place, which means these collaterals, are only the allowed ones considered as valid, this piority collateral list can also be removed. This issue occurs due to differences between the variable used for checking the liquidation status of an account and the actual collateral that are considered valid for liquidations.
Lets summarize

• The protocol has a liquidation priority list, which are the only ones users can deposit

• These are the only ones considered valid and can also be removed

• When these are removed it does not check if users have these collateral in use in their marginCollateralBalanceX18

• This collateral remains and when getMarginBalanceUsd is called it uses these collateral to calculate the totalMarginBalance, even though the priority list does not contain this anymore

• The isLiquidatable check inside in checkLiquidatableAccounts and liquidateAccounts will both fail even though it should pass and would prevent liquidations

Vulnerability Details

To simplfy the details, snippets of the summarized version codes would be shown to prove the point

When these list are modified users still have the collateralType inside their marginCollateralBalanceX18 enumerable set, with their balance still intact
Now in the liquidateAccount a call to getMarginBalanceUsd is made and retrives the margin balance as shown below

To Prove this, we need to show how the liquidations work and which collaterals are actually valid for it. When the deductAccountMargin is called, inside the liquidation process we get a glimpse of how it works and what exactly is important to it

As seen above when it actually wants to liquidate it uses the collateralLiquidationPriority list to iterate over collateralls to liquidate from which might not have the collateralType that has being removed but still exists in marginCollateralBalanceX18, this leads to the check below failing when it should not

The maintenance margin will be correct, but the marginBalance will be incorrect which will lead to this check failing

A Real life exploit scenario, would be a user spotting that a priority collateralType is about to be removed and adding that collateral type which allows them to avoid liquidations, but a user doesn't have to be malicious for this to happen as it would happen because some users would still hold that collateral which would also help them prevent liquidations when the situation happens

Impact

Liquidations are one of the main components of perpetuals protocol, and hindering the ability to liquidate may cause losses and make the protocol ineffective, furthermore the protocol collects liquidation fee which will be affected when liquidations cannot occur.

Tools Used

Manual Review

Recommendations

Implement a check to correctly assert that collateralType being removed is not in use anywhere in the protocol, additionally margin balance should only be gotten from list of priority collateralType to correctly reflect the state.