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

During liquidation, positive PnL is not awarded to the user

glightspeed2

Summary

During liquidation, the protocol deducts account margin without awarding any positive PnL to user. This can result an unfair over-liquidation of collateral asset.

Vulnerability Details

LiquidationBranch.liquidateAccounts can be summarized as the following steps:

  • Calculate requiredMaintenanceMarginUsd and accountTotalUnrealizedPnlUsd for given account

  • Calculate marginBalanceUsd for the account

  • Check if marginBalanceUsd < requiredMaintenanceMarginUsd + liquidationFeeUsd

  • For all active markets, close the position and update funding rate and skews

  • Deduct accountTotalUnrealizedPnlUsd.abs() + requiredMaintenanceUsd from account's margin

where

To make things simpler, let's assume an account has only one collateral type and liquidationFeeUsd = 0

Consider the following scenario:

  • accountTotalUnrealizedPnlUsd = 50

  • collateralInUsd = 1000

  • requiredMaintenanceMarginUsd = 900

  • loanToValue = 0.8

The account is liquidable because:

The amount to deduct from account is:

So, after liquidation, the account is left with

worth of collateral and no position. Since all position is lost, the user cannot claim 50 USD worth of accountTotalUnrealizedPnlUsd anymore.

Thus, the user lost 50 USD worth of accountTotalUnrealizedPnlUsd during liquidation.

Because taking positive PnL into account, remaining asset after liquidation should be:

Impact

User's positive PnL is not realized during liquidation, and user gets their collateral liquidated more than needed.

Tools Used

Manual Review

Recommendations

Introduce the same postive PnL logic as in SettlementBranch._fillOrder

An alternative approach is not to withdraw positive PnL from trade account:

diff --git a/src/perpetuals/branches/LiquidationBranch.sol b/src/perpetuals/branches/LiquidationBranch.sol
index 6aa4db0..e4eb9ab 100644
--- a/src/perpetuals/branches/LiquidationBranch.sol
+++ b/src/perpetuals/branches/LiquidationBranch.sol
@@ -221,7 +221,10 @@ contract LiquidationBranch {
// checks would fail
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}
-
+ UD60x18 pnlUsdX18 = ctx.requiredMaintenanceMarginUsdX18;
+ if (ctx.accountTotalUnrealizedPnlUsdX18.lt(SD59x18_ZERO)) {
+ pnlUsdX18 = pnlUsdX18.add(ctx.accountTotalUnrealizedPnlUsdX18.abs().intoUD60x18());
+ }
// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin(
@@ -231,9 +234,7 @@ contract LiquidationBranch {
orderFeeRecipient: address(0),
settlementFeeRecipient: perpsEngineConfiguration.liquidationFeeRecipient
}),
- pnlUsdX18: ctx.accountTotalUnrealizedPnlUsdX18.abs().intoUD60x18().add(
- ctx.requiredMaintenanceMarginUsdX18
- ),
+ pnlUsdX18: pnlUsdX18,
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18,
marketIds: ctx.activeMarketsIds,
​
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.