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

SEV 2: Difference in collateral tokens used for margin deduction and margin balance calculation allows trader to trade risk-free

Severity: High

Summary

The TradingAccount::deductAccountMargin function is used to deduct fees and losses from user collateral. The deductAccountMargin uses GlobalConfiguration::collateralLiquidationPriority as the token list and deducts only from trader's balance of these tokens.

The TradingAccount::getMarginBalanceUsd is used to calculate user margin balance and it uses TradingAccount::marginCollateralBalanceX18 list to compute trader's balance.

The protocol does not allow traders to deposit tokens which are not in collateralLiquidationPriority. However, if a token was removed from collteralLiquidationPriority then traders can have non-zero balance of the removed token from previous deposits and the token will be present in the marginCollateralBalanceX18 list.

If the trader has enough balance in the removed token to cover for margin requirements in a perp market then the trader can update their positions such that the removed token's balance covers the margin requirements and remove their deposits in rest of the tokens. As a result,

  • The trader will have open positions

  • Zero collateral balance of all the tokens except for the removed token

  • The balance of the removed token is used as the margin

Because the trader has 0 balance for all tokens in collateralLiquidationPriority, the deductAccountMargin returns without deducting any collateral.

The deductAccountMargin function is used to deduct settlement fee, order fee, and loss when settling orders and to deduct trader's collateral upon liquidation.

This allows trader to

  • Perform trades without paying fees.

  • Make their losses zero by opening and closing positions regularly

  • The account becomes liquidatable if and only if the price of the removed token declines to the point where the margin requirements for minimum trade size are no longer met. Liquidating the account will close the positions with zero deductions.

  • The trader can withdraw the tokens or wait for price of that token to increase and open the positions again. Trader can repeat this till the token is added back again to the collateralLiquidationPriority list.

The trader will receive any profits made in the process allowing them to make profits with zero risk and deductions.

Vulnerability Details

The GlobalConfiguration::collateralLiquidationPriority is a list, global to the protocol and represents the order for performing deductions from user's collaterals.

Definition of collateralLiquidationPriority: GlobalConfiguration.sol#L51

The list is mainly used by the TradingAccount::deductAccountMargin function. The deductAccountMargin function is intended to deduct fees and losses from the trader's collateral.

The function iterates over the list and checks if the trader has any balance in those tokens. The function only deducts from these token balances. If the trader has zero balance in these tokens then the function will not perform any deductions.

Snippet of TradingAccount::deductAccountMargin responsible for iterating over trader token balances and deductions: TradingAccount.sol#L508-L520

It should not be possible to create order with zero balance in these tokens. However, the protocol computes margin balance using a different list and it is possible for this list to have tokens which are not in collateralLiquidationPriority.

The TradingAccount::getMarginBalanceUsd function uses the TradingAccount::marginCollateralBalanceX18 list to compute margin balance. This list contains all the tokens the trader has made deposits in.

Definition of TradingAccount::getMarginBalanceUsd: TradingAccount.sol#L178-L196

The protocol does not allow to make deposits in tokens which are not in collteralLiquidationPriority list:

TradingAccountBranch.sol#L343

However, a token which was previously in the collateralLiquidationPriority list can be removed. The function GlobalConfiguration::removeCollateralFromLiquidationPriority can be used by the protocol owner to remove a token from collateralLiquidationPriority list

Definition of GlobalConfiguration::removeCollateralFromLiquidationPriority: GlobalConfigurationBranch.sol#L271-L278

Once a token is removed from liquidation priority list then the marginCollateralBalanceX18 list of traders with previous deposits in the removed token will be different. This can be leveraged by the trader to make profit with zero risk and deductions.

Exploit Scenario:

  • collateralLiquidationPriority = [Ta, Tb]

  • Trader Eve deposits 100k USD value in token Tb and opens a position.

  • marginCollateralBalanceX18 list of Eve is [Tb]

  • Eve has opened long position in BTC market.

  • Protocol owner removed token Tb from collateralLiquidationPriority

  • collateralLiquidationPriority = [Ta]

  • Eve has made loss from the position.

  • Eve creates an order with minTradeSize and the protocol attempts to deduct loss, fees from the collateral.

    • The protocol cannot perform deductions and Eve's position is marked as if loss was paid.

  • Eve's account becomes liquidatable and the liquidator closes the position with zero deductions

    • because Intersection(collateralLiquidationPriority, marginCollateralBalanceX18) is null.

  • Eve still has deposited tokens. Eve repeats the process waiting for profit while making her losses zero.

  • Eve can create new positions as long as Tb balance meets margin requirements for minTradeSize of a market

Impact

Traders can trade with zero deductions on settlement and liquidations if a token was removed from the collateralLiquidationPriority list.

Tools Used

Manual Review

Recommendations

  • Use the collateralLiquidationPrioriy list in the getMarginalUsd function.

  • In order to avoid system wide liquidations upon removing token from collateralLiquidationPriority, allow users time to add collateral in other tokens for a certain period of time from the removal. This can be implemented by adding a mapping to the GlobalConfiguration::Data which stores removed tokens and the time of removal. Account for the trader's balance of removed tokens in the getMarginUsd function for that fixed period.

  • Update the deductAccountMargin function to deduct from the marginCollateralBalanceX18 tokens if the tokens in collateralLiquidationPriority do not cover the required deductions.

Updates

Lead Judging Commences

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

removal of collateral types from the liquidation priority list will affect affect the liquidation process of the existing positions

Appeal created

s3v3ru5 Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
s3v3ru5 Submitter
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

removal of collateral types from the liquidation priority list will affect affect the liquidation process of the existing positions

Support

FAQs

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