DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Lack of proper Margin/balance validation during the offchain execution can cause a user to be liquidated immediately he reduces his position

Summary

The report addresses a critical vulnerability in the offchain execution process that lacks proper validation, leading to potential immediate liquidation of a user's position when they reduce their position size. The vulnerability arises due to the absence of a buffer check in the offchain validation process, unlike the onchain validation, which ensures that users are not at risk of liquidation when adjusting their positions.

Vulnerability Details

In the current implementation, whenever a user creates an onchain order, the new order is validated against their old position's required maintenance margin. This validation ensures that even if the user is reducing their position, they remain above their former margin and maintain a buffer between the former margin and the new margin, preventing immediate liquidation.

However, in the offchain execution, the validation fails to ensure that the user's new position includes this buffer. This oversight allows a user to reduce their position at the risk of being liquidated.

**Code Snippet with Practical Examples:**

1. User A opens a $3000 position:

- Initial margin: $2000

- Required margin: $1000

- Balance: $3000

- PnL: -$1200

2. User A decides to reduce his position by $1000:

- New position size: $3000 - $1200 = $1800

- Below initial margin: $2000

3. User opens a trade to reduce his debt by half to $800:

- This triggers a market order and the following checks are made:

(requiredInitialMarginUsdX18, requiredMaintenanceMarginUsdX18, ctx.accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, ctx.sizeDeltaX18);
@audit>> new balance>>> marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(ctx.accountTotalUnrealizedPnlUsdX18);
@audit>> previous margin>>> (, ctx.previousRequiredMaintenanceMarginUsdX18,) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18\_ZERO);
@audit >> ensures buffer >>> if (TradingAccount.isLiquidatable(ctx.previousRequiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
revert Errors.AccountIsLiquidatable(tradingAccountId);
}

4. Comparison of balance with previous required maintenance margin:

- Previous margin: $1000

- New balance: $1800 - $1000 = $800

- Transaction reverts because the balance is less than the required maintenance margin (800<1000).

5. If the user reduces his position by $800:

- New position: $3000 - $800 = $2200

- New initial margin: $1465 (66.6% of position size)

- New maintenance margin: $732

- New balance: $2200 - $1200 = $1000

- Subsequent checks pass, allowing the position reduction.

6. Lack of validation in offchain execution:

- Example: User A sends an offchain order to reduce his position by $1068:

- New position: $3000 - $1068 = $1932

- New initial margin: $1465

- New maintenance margin: $732

- New balance: $1932 - $1068 = $732

ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
@audit >> offchain only validate position against new margin>> tradingAccount.validateMarginRequirement(
ctx.requiredMarginUsdX18,
tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18)
);

- This passes, and the user reduces their position below the former required maintenance margin, to his new required maintenance margin risking liquidation.

Impact

The lack of proper validation during offchain execution can cause users to be liquidated immediately upon reducing their position. This poses a significant financial risk to users.

Tools Used

- Solidity

- Onchain and offchain validation logic analysis

- Practical trade examples

Recommendations

To mitigate this vulnerability, it is essential to add the necessary checks in the offchain order validation process before execution is processed. This includes ensuring that the user's new position maintains a buffer above the former required maintenance margin, similar to the onchain validation process.

add this check

++ (requiredInitialMarginUsdX18, requiredMaintenanceMarginUsdX18, ctx.accountTotalUnrealizedPnlUsdX18) =
++ tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, ctx.sizeDeltaX18);
++ @audit>> new balance>>> marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(ctx.accountTotalUnrealizedPnlUsdX18);
++ @audit>> previous margin>>> (, ctx.previousRequiredMaintenanceMarginUsdX18,) =
++ tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
++ @audit >> ensures buffer >>> if (TradingAccount.isLiquidatable(ctx.previousRequiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
++ revert Errors.AccountIsLiquidatable(tradingAccountId);
++ }

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`isLiquidatable` check missing in `_fillOrder()`

Support

FAQs

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