Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

incorrect calculations in function liquidateAccounts

kwakudr

Summary

The calculation of pnlUsdX18 used in deducting the account margin during liquidation is incorrect due to the use of abs() (absolute value) on the unrealized PnL.

Vulnerability Details

// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin(
TradingAccount.DeductAccountMarginParams({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: perpsEngineConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: perpsEngineConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: ctx.accountTotalUnrealizedPnlUsdX18.abs().intoUD60x18().add(
ctx.requiredMaintenanceMarginUsdX18
),
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18,
marketIds: ctx.activeMarketsIds,
accountPositionsNotionalValueX18: ctx.accountPositionsNotionalValueX18
})
);

The pnlUsdX18 is intended to represent the total amount the trader owes due to unrealized losses and required maintenance margin.

By applying abs() to accountTotalUnrealizedPnlUsdX18, we lose the sign of the PnL. Both gains and losses become positive values.

Adding the required maintenance margin to this absolute value can lead to an incorrect total, either undercharging or overcharging the trader.

The abs() function makes the loss positive.

Adding this positive value to the required maintenance margin inflates the amount deducted.

The trader is overcharged, leading to negative account balances.

Impact

If the trader has a positive unrealized PnL (gain):

The abs() function keeps the gain positive. Adding it to the required maintenance margin overstates the amount to deduct. The trader's profit is wrongly considered a liability.

Tools Used

Manual Review

Recommendations

SD59x18 totalLossUsdX18 = ctx.requiredMaintenanceMarginUsdX18.intoSD59x18().sub(ctx.accountTotalUnrealizedPnlUsdX18);
​
// Ensure totalLossUsdX18 is non-negative
if (totalLossUsdX18 < SD59x18(0)) {
totalLossUsdX18 = SD59x18(0);
}
​
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin(
TradingAccount.DeductAccountMarginParams({
pnlUsdX18: totalLossUsdX18.intoUD60x18(),
})
);
Updates

Lead Judging Commences

inallhonesty Lead Judge
6 months ago
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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