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

LiquidationBranch::liquidateAccounts improperly handles Unrealized Pnl during liquidation.

sohrabhind

Relevant GitHub Links

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

Summary

LiquidationBranch::liquidateAccounts add accountTotalUnrealizedPnlUsdX18 to marginBalance only for working data which used for checking isLiquidatable, and when later deductAccountMargin is called it deduct required margin and fees from old margin without considering the unrealized PnL.

Vulnerability Details

In the liquidateAccounts function, the margin balance is augmented(for working data only) by accountTotalUnrealizedPnlUsdX18 to determine if the account is liquidatable. However, this augmented balance is not used consistently during the actual liquidation process. Specifically:

  • The augmented balance is used to check if the account meets the liquidation criteria.

  • During the deduction of margin and fees in the deductAccountMargin call, the deduction is performed on the old margin balance, ignoring the unrealized PnL.

// get account's required maintenance margin & unrealized PNL
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
// get then save margin balance into working data
ctx.marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
​
// if account is not liquidatable, skip to next account
// account is liquidatable if requiredMaintenanceMarginUsdX18 > ctx.marginBalanceUsdX18
if (!TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, ctx.marginBalanceUsdX18)) {
continue;
}
​
// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: requiredMaintenanceMarginUsdX18,
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});

Impact

High

Tools Used

Manual

Recommendations

Update account margin balance before calling deductAccountMargin.

Updates

Lead Judging Commences

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

deductAccountMargin() treats `required maintenance margin` as the `unrealized PnL` because it uses `requiredMaintenanceMarginUsdX18` in place of `pnlUsdX18`

Support

FAQs

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