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

Potential incorrect `maintenance margin` deduction in `liquidateAccounts()`

tigerfrake

Summary

The liquidateAccounts()function incorrectly assigns requiredMaintenanceMarginUsdX18 to the pnlUsdX18 parameter in the deductAccountMargin() function, which should represent the account's unrealized PnL. This misassignment can lead to incorrect margin deductions.

Vulnerability Details

The current implementation of the deductAccountMargin() function call is as follows:

// 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, // @audit-info Incorrect parameter
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});

The issue lies in the incorrect parameter assignment for the pnlUsdX18 parameter in the deductAccountMargin() function call. The pnlUsdX18 parameter is supposed to represent the total unrealized PnL of the account, but the code is using requiredMaintenanceMarginUsdX18 instead.

Here is the deductAccountMargin() Signature:

//...
>> /// @param pnlUsdX18 The total unrealized PnL of the account.
//...
function deductAccountMargin(
Data storage self,
FeeRecipients.Data memory feeRecipients,
>> UD60x18 pnlUsdX18,
UD60x18 settlementFeeUsdX18,
UD60x18 orderFeeUsdX18
)

A seen above:

pnlUsdX18 is the total unrealized PnL of the account.

Therefore, by using requiredMaintenanceMarginUsdX18 in place of pnlUsdX18, the function is essentially treating the required maintenance margin as the unrealized PnL, which is incorrect.

Impact

Incorrect margin deductions can severely impact a user's financial health, either by liquidating more collateral than necessary or by leaving insufficient collateral to cover potential losses. It can also lead to liquidity issues and undermine the protocol's integrity.

Tools Used

Manual Review

Recommendations

To ensure accurate margin deductions, the pnlUsdX18 parameter should be set to the account's unrealized PnL.

// get account's required maintenance margin & unrealized PNL
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
//...
​
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
- pnlUsdX18: requiredMaintenanceMarginUsdX18,
+ pnlUsdX18: accountTotalUnrealizedPnlUsdX18.intoUD60x18(),
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.