Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

getMarginBalanceUsd() Collateral removed from collateralLiquidationPriority should not be counted in the total collateral balance

0x1982us

Summary

getMarginBalanceUsd() uses TradingAccount.marginCollateralBalanceX18 to calculate the current collateral balance.
If collateral is removed collateralLiquidationPriority is still counted in the total balance.
This way the user can still use the removed collateral to add a new position, but this collateral will not be liquidated in the event of a liquidation.

Vulnerability Details

TradingAccount.getMarginBalanceUsd() is used to actuate the current collateral balance.

function getMarginBalanceUsd(
Data storage self,
SD59x18 activePositionsUnrealizedPnlUsdX18
)
...
@> uint256 cachedMarginCollateralBalanceLength = self.marginCollateralBalanceX18.length();
​
// iterate over every collateral account has deposited
for (uint256 i; i < cachedMarginCollateralBalanceLength; i++) {
// read key/value from storage for current iteration
(address collateralType, uint256 balance) = self.marginCollateralBalanceX18.at(i);
​

This method uses TradingAccount.data.marginCollateralBalanceX18 to loop through and calculate to get the total balance.

TradingAccount.deductAccountMargin() is used in liquidation to deduct the collateral.

function deductAccountMargin(
...
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
​
// cache collateral liquidation priority length
@> uint256 cachedCollateralLiquidationPriorityLength = globalConfiguration.collateralLiquidationPriority.length();
​
// loop through configured collateral types
for (uint256 i; i < cachedCollateralLiquidationPriorityLength; i++) {
// get ith collateral type
@> address collateralType = globalConfiguration.collateralLiquidationPriority.at(i);
​
// fetch storage slot for this collateral's config config
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
​

This method uses globalConfiguration.collateralLiquidationPriority to loop through and deduct the corresponding collateral equivalent from it

So let's say collateral_C is removed from collateralLiquidationPriority, but the user has previously had collateral_C deposited .

Then the user's

marginCollateralBalanceX18 = [collateral_C]

But

globalConfiguration.collateralLiquidationPriority = [collateral_A,collateral_B]

it is used to increase the position using the valuation of collateral_C.

But there is no liquidation risk because collateral_C is not included in collateralLiquidationPriority.

Impact

Increase the position using the removed collateral, but without the risk of liquidation.

Tools Used

Recommendations

getMarginBalanceUsd() use globalConfiguration.collateralLiquidationPriority()

or

deductAccountMargin() to prioritize using globalConfiguration.collateralLiquidationPriority() and if that's not enough then TradingAccount.data. marginCollateralBalanceX18.

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

Appeal created

0x1982us Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
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.