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

Users can have positions which are only/primarily backed by unrealized PnL which is can cause losses for the protocol.

Summary

When a user wants to interact with their trading account via orders or withdrawing collateral their margin balance is always calculated in order to allow that interaction to happen without putting the protocol at risk. But if a user has a high enough profit from their open positions they can withdraw all their collateral and simply use their unrealized PnL as collateral for their positions. That is dangerous as the protocol will not be able to collect any fees from the account if the positions start suffering losses and become liquidatable.

Vulnerability Details

Because the trader's margin balance is always calculated using TradingAccount::getMarginBalanceUsd and then compared with requiredInitialMargin or requiredMaintenanceMargin in order to determine if their interaction with the contract is safe a user can have their position backed only by PnL. If the user's position is profitable enough to sustain itself the user can withdraw all of their margin and avoid paying liquidation fee if their account is to be liquidated.

Example:

Alice opens a 2x long position on USD/BTC with 1000$ of collateral and the price of BTC is 2000$ and let's say her maintenance margin is 500$. After some time the price of BTC goes to 2600$ and Alice has an unrealized profit of 600$. Now Alice can withdraw her initial 1000$ of collateral and protocol will allow her to do that since the result of TradingAccount::getMarginBalanceUsd would be 1600 and 1600-1000 = 600 which is more than the required maintenance margin. Now if the price of BTC falls to 2450$ Alice's margin balance would be 450$ which is less than her maintenance margin and she would be liquidated. That is where the problem comes as the protocol will not be able to receive the liquidation fee or even the value of the liquidation as under normal circumstance the required maintenance margin would be subtracted from Alice's account but since she has 0$ in her account margin and only unrealized profits which cannot be payed to the protocol.

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

Impact

Protocol suffers losses due to the inability to collect liquidation fee.

Tools Used

Manual Review
VS Code

Recommendations

Do not allow accounts to withdraw all of their collateral while they have open positions. There must be enough collateral to cover for a potential liquidation fee payment.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

A user with a big enough positive PnL can withdraw his margin and avoid liquidation fee in case they get liquidated

Support

FAQs

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