Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Lack of the `liquidationFeeUsd` in the calculation of the `isLiquidatable`, which lead the liquidation threshold (`isLiquidatable`) to always being miscalculated

0xmuxyz

Summary

Within the TradingAccount#isLiquidatable(), the liquidationFeeUsd is supposed to be used in the calculation of the isLiquidatable.
However, within the TradingAccount#isLiquidatable(), the liquidationFeeUsd is not used in the calculation of the isLiquidatable.

This lead the liquidation threshold (isLiquidatable) to always being miscalculated.

Vulnerability Details

Within the LiquidationBranch#checkLiquidatableAccounts(), the TradingAccount#isLiquidatable() would be called like this:
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol#L82

function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
​
// iterate over active accounts within given bounds
for (uint256 i = lowerBound; i < upperBound; i++) {
...
// account can be liquidated if requiredMargin > marginBalance
if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) { ///<------- @audit
liquidatableAccountsIds[i] = tradingAccountId;
}
}

Within the TradingAccount#isLiquidatable(), whether or not the account is liquidatable would be calculated and validated.
Then, the result of validation will be returned like this:
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/TradingAccount.sol#L350

/// @notice Checks if the account is liquidatable.
/// @param requiredMaintenanceMarginUsdX18 The required maintenance margin in USD.
/// @param marginBalanceUsdX18 The account's margin balance in USD.
function isLiquidatable(
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 marginBalanceUsdX18
)
internal
pure
returns (bool)
{
return requiredMaintenanceMarginUsdX18.intoSD59x18().gt(marginBalanceUsdX18); ///<------- @audit
}

According to the "Liquidation Criteria" in the document, the liquidationFeeUsd is supposed to be used in the calculation of the isLiquidatable like this:

isLiquidatable = requiredMM + liquidationFeeUsd >= marginBalanceUsd

However, within the TradingAccount#isLiquidatable(), the liquidationFeeUsd is not used in the calculation of the isLiquidatable like this:

return requiredMaintenanceMarginUsdX18.intoSD59x18().gt(marginBalanceUsdX18);

Impact

This lead the isLiquidatable to always be miscalculated.

This means that the liquidation threshold would always be miscalculated.
And therefore, both impact and likelihood is "High". Hence, this severity can be "High".

Tools Used

  • Foundry

Recommendations

Within the TradingAccount#isLiquidatable(), consider adding the liquidationFeeUsd to the calculation of the isLiquidatable like this:

function isLiquidatable(
UD60x18 requiredMaintenanceMarginUsdX18,
+ SD59x18 liquidationFeeUsdX18
SD59x18 marginBalanceUsdX18
)
internal
pure
returns (bool)
{
+ return (requiredMaintenanceMarginUsdX18.intoSD59x18() + liquidationFeeUsdX18).gt(marginBalanceUsdX18);
- return requiredMaintenanceMarginUsdX18.intoSD59x18().gt(marginBalanceUsdX18);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Liquidation doesn't take the liquidation fee in consideration inside the isLiquidatable check

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!