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

Wrong input paramater when checking account is liquidatable

AllTooWell

Summary

Wrong input paramater when checking account is liquidatable. The check that account can be liquidatable could get wrong result.

Vulnerability Details

OrderBranch::simulateTrade:

(requiredInitialMarginUsdX18, requiredMaintenanceMarginUsdX18, ctx.accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, ctx.sizeDeltaX18);
​
// use unrealized PNL to calculate & output account's margin balance
marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(ctx.accountTotalUnrealizedPnlUsdX18);
{
// get account's current required margin maintenance (before this trade)
(, ctx.previousRequiredMaintenanceMarginUsdX18,) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
// prevent liquidatable accounts from trading
@ if (TradingAccount.isLiquidatable(ctx.previousRequiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) { // @audit-check marginBalance is unaltered before trade and after trade
revert Errors.AccountIsLiquidatable(tradingAccountId);
}
}

From the above code we can see the first paramater of isLiquidatable function is obtained by getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO) which means the paramater is before trade.
For the second paramater ?
The second parameter is obtained from marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(ctx.accountTotalUnrealizedPnlUsdX18);, and ctx.accountTotalUnrealizedPnlUsdX18 is derived from tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, ctx.sizeDeltaX18). Therefore, it indicates that the margin balance takes into account the PnL after trade, representing the second paramater is after trade.
So the result of isLiquidatable can be wrong because of two inconsistent paramater.

Impact

The check that account can be liquidatable could get wrong result and lead to liquidatable accounts trading.

Tools Used

manual

Recommendations

Two paramater of isLiquidatable use the value before trade.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

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

The protocol compares the margin balance after the trade with current required maintenance margin to determine if the trading account is currently liquidatable

Support

FAQs

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

Give us feedback!