DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: high
Valid

Liquidation will almost always withdraw less margin than needed

Summary

Liquidation will always give less than requiredMaintenanceMarginUsdX18 to Zaros, because it doesn't want a certain margin to exist and what's left is transferred.

Vulnerability Details

On liquidation, it checks if requiredMaintenanceMarginUsdX18 > marginBalanceUsdX18 (margin + PnL + funding) and if so, deducts requiredMaintenanceMarginUsdX18 + liquidation fee from the margin balance of the tradingAccount. But in almost all cases, the margin that will be paid to the protocol will be less than the margin balance of the tradingAccount and they will never receive the required MaintenanceMarginUsdX18 + liquidation fee.

As you can see it tries to send requiredMaintenanceMarginUsdX18 to marginCollateralRecipient and liquidationFeeUsdX18 to liquidationFeeRecipient. TradingAccount.deductAccountMargin() is implemented to send the fees first and then the requiredMaintenanceMarginUsdX18.

138: (, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
139: tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
140:
141: // get then save margin balance into working data
142: ctx.marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
143:
144: // if account is not liquidatable, skip to next account
145: // account is liquidatable if requiredMaintenanceMarginUsdX18 > ctx.marginBalanceUsdX18
146: if (!TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, ctx.marginBalanceUsdX18)) {
147: continue;
148: }
149:
150: // deduct maintenance margin from the account's collateral
151: // settlementFee = liquidationFee
152: ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
153: feeRecipients: FeeRecipients.Data({
154: marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
155: orderFeeRecipient: address(0),
156: settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
157: }),
158: pnlUsdX18: requiredMaintenanceMarginUsdX18,
159: orderFeeUsdX18: UD60x18_ZERO,
160: settlementFeeUsdX18: ctx.liquidationFeeUsdX18
161: });

These are the possible cases where a liquidation will occur and in almost all cases they will not send the full desired amount. This value can also get smaller and smaller and even reach 0 because TradingAccount.deductAccountMargin() only sends what is left over.

Note: liquidationFee = 10

  1. requiredMaintenanceMargin = 100, marginBalance = 90 (Margin = 105, PnL = -20, funding = 0), in this case, due to the negative PnL, the position is liquidatable. But the trading account has only 105 margin and 10 will be sent to liquidationFeeRecipient, the remaining 95 will be sent to marginCollateralRecipient.

  2. requiredMaintenanceMargin = 100, marginBalance = 99 (Margin = 90, PnL = 9, funding = 0), in this case the price of the margin collateral has dropped and again 10 will be sent to liquidationFeeRecipient, the remaining 80 will be sent to marginCollateralRecipient even if they wanted 100

  3. requiredMaintenanceMargin = 100, marginBalance = 90 (Margin = 110, PnL = -10, funding = 0), this is the only case when both will be satisfied, when margin > requiredMaintenanceMargin + fee.

Impact

Zaros will not receive the requested margin when liquidating a position.

Tools Used

Manual Review

Recommendations

Implement a check to ensure at least amount that is required to be sent upon liquidation.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 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.