Zaros Part 1

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol#L137-L161

Summary

LiquidationBranch::liquidateAccounts does not account for the liquidationFeeUsdX18 when determining if an account is liquidatable.
According to the intended logic, an account should be liquidated if requiredMaintenanceMargin + liquidationFeeUsd is greater than to the marginBalanceUsd. However, the current implementation only checks if requiredMaintenanceMargin exceeds the marginBalanceUsd, omitting the liquidation fee from the equation.

Vulnerability Details

In the liquidateAccounts function, the isLiquidatable check is performed using only the requiredMaintenanceMarginUsdX18 and the marginBalanceUsdX18:

The isLiquidatable function should include the liquidationFeeUsdX18 in its calculations to correctly determine if an account can be liquidated.

Impact

Accounts that should be liquidated based on the combined required maintenance margin and liquidation fee remain active, and when they got liquidated they may not have sufficient collatral.

Tools Used

Manual

Recommendations

To resolve this issue, update the isLiquidatable function to include the liquidation fee in the liquidation condition. The modified isLiquidatable function should look like this:

Ensure that the liquidateAccounts function passes the liquidationFeeUsdX18 when calling isLiquidatable: