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

Removing a collateral type from CollateralLiquidationPriority will lead to an overestimation of users margin balance

Impact

When a collateral type is removed from the CollateralLiquidationPriority, its loanToValue is not set to 0. This means that collateral of the removed type will still be included in the margin balance calculation of a user account, even though it is no longer accepted for paying fees and losses of an account. This leads to an overestimation of the margin balance of an account and can result in bad debt for the protocol.

Proof of Concept

CollateralLiquidationPriority is an EnumerableSet of collateral addresses which are accepted for paying fees or accrued losses of an account. When a position of an account is filled or an account is liquidated, the deductAccountMargin() function in TradingAccount.sol iterates over the collateralLiquidationPriority to pay fees or negaitve PnL of an account by sending the collaterals deposted by the user to the feerecipients.
To determine if an account is liquidatable or meets the margin requirements for filling a position, the required margin is compared to the marginBalance of the account. The marginBalance of the account is calculated by iterating over each collateral deposited to the account and determining its value by multiplying the deposited collateral amount with its price and with its loanToValue (% of value to be considdered as margin):

function getMarginBalanceUsd(
Data storage self,
SD59x18 activePositionsUnrealizedPnlUsdX18
)
internal
view
returns (SD59x18 marginBalanceUsdX18)
{
...
for (uint256 i; i < cachedMarginCollateralBalanceLength; i++) {
...
// calculate the collateral's "effective" balance as:
// collateral_price * deposited_balance * collateral_loan_to_value_ratio
UD60x18 adjustedBalanceUsdX18 = marginCollateralConfiguration.getPrice().mul(ud60x18(balance)).mul(
ud60x18(marginCollateralConfiguration.loanToValue)
);
// add this account's "effective" collateral balance to cumulative output
marginBalanceUsdX18 = marginBalanceUsdX18.add(adjustedBalanceUsdX18.intoSD59x18());
}
...
}

However, if a collateral type is removed from this CollateralLiquidationPriority and is no longer usable for paying fees or losses of an account, its loanToValue is not reset to 0, causing it to still be considered in the margin balance calculation. This results in a marginBalance which is to high and can result in losses for the protocol.

Scenario

  1. Collateral of type A and type B are included in the CollateralLiquidationPriority

  2. A user deposits collateral of type A and type B and opens a position

  3. The admin removes collateral type A from the CollateralLiquidationPriority.

  4. The user should be liquidatable becasue the deposited collateral of type B does not cover the required margin but he is not becasue the user’s margin balance is still calculated including collateral type A.

  5. The user’s position incurs losses, but the system cannot deduct the required margin due to the overestimation, leading to a loss for the protocol

Recommended Mitigation Steps

To prevent this issue, ensure that when a collateral type is removed from the CollateralLiquidationPriority, the values of its MarginCollateralConfiguration are set to 0. This can be done by modifying the removeCollateralFromLiquidationPriority() function in GlobalConfiguration.sol:

function removeCollateralFromLiquidationPriority(address collateralType) external onlyOwner {
if (collateralType == address(0)) {
revert Errors.ZeroInput("collateralType");
}
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
globalConfiguration.removeCollateralFromLiquidationPriority(collateralType);
+ MarginCollateralConfiguration.configure( collateralType, 0, 0, 0, 0, 0);
emit LogRemoveCollateralFromLiquidationPriority(msg.sender, collateralType);
}

This ensures that the removed collateral type is no longer considered in the margin balance calculation even if a user has deposited the collateral before.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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.