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

Incorrect margin deduction logic due to misuse of parameters in the liquidation process

whitehat777

Summary

The LiquidationBranch contract in Zaros' perpetual trading system is responsible for managing liquidations of trading accounts. A critical issue has been identified in the liquidateAccounts function where the deductAccountMargin function is called with an incorrect parameter. Specifically, the pnlUsdX18 parameter is incorrectly set to the value of requiredMaintenanceMarginUsdX18 instead of the actual unrealized profit and loss (accountTotalUnrealizedPnlUsdX18). This discrepancy could lead to inaccuracies in margin deductions and liquidation processes.

Vulnerability Details

Parameter Mismatch:

  • In the LiquidationBranch::liquidateAccounts() function, the deductAccountMargin function is invoked with the pnlUsdX18 parameter set to requiredMaintenanceMarginUsdX18, which is not appropriate.

LiquidationBranch.sol#L152-L161

function liquidateAccounts(uint128[] calldata accountsIds) external {
...
// 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;
}
​
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
>>> pnlUsdX18: requiredMaintenanceMarginUsdX18, // @audit Incorrect parameter
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});
...
​

Intended Usage:

  • The pnlUsdX18 parameter should reflect the account's actual unrealized profit and loss (accountTotalUnrealizedPnlUsdX18), not the required maintenance margin (requiredMaintenanceMarginUsdX18).

TradingAccount.sol#L484

/// @notice Deducts the account's margin to pay for the settlement fee, order fee, and realize the pnl.
/// @param self The trading account storage pointer.
/// @param feeRecipients The fee recipients.
>>> /// @param pnlUsdX18 The total unrealized PnL of the account.
/// @param settlementFeeUsdX18 The total settlement fee to be deducted from the account.
/// @param orderFeeUsdX18 The total order fee to be deducted from the account.
/// @return marginDeductedUsdX18 The total margin deducted from the account.
function deductAccountMargin(
...

Impact

Using requiredMaintenanceMarginUsdX18 instead of accountTotalUnrealizedPnlUsdX18 for pnlUsdX18 will lead to incorrect margin deductions. This can cause either excessive or insufficient deductions, potentially resulting in erroneous liquidations and financial instability for traders.

Tools Used

Manual code review

Recommendations

Parameter Correction: modify the call to deductAccountMargin to use the correct parameter for unrealized PnL. Replace requiredMaintenanceMarginUsdX18 with accountTotalUnrealizedPnlUsdX18 in the pnlUsdX18 parameter.

ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
- pnlUsdX18: requiredMaintenanceMarginUsdX18 // @audit Incorrect parameter
+ pnlUsdX18: accountTotalUnrealizedPnlUsdX18.intoUD60x18(), // Corrected parameter
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});
Updates

Lead Judging Commences

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