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

isLiquidateable check doesn't contain the liquidation fees

SBSecurity

Summary

requiredMaintenanceMargin does not take into account the liquidationFee, which will prevent some accounts from being liquidated.

Vulnerability Details

When is checked if a user is for liquidation, it gets the requiredMaintenanceMargin for the specific position and then compares it to the trader's marginBalance (margin + PnL + funding).

The account can be liquidated if requiredMaintenanceMargin > marginBalance

function isLiquidatable(UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 marginBalanceUsdX18) internal pure returns (bool)
{
return requiredMaintenanceMarginUsdX18.intoSD59x18().gt(marginBalanceUsdX18);
}

But since liquidation has a liquidationFee, requiredMaintenanceMargin must include the fee before checking if it is liquidatable, since every liquidation must pay the liquidationFee.

Consider this example:

requiredMaintenanceMargin = 100, marginBalance = 101, liquidationFee = 5 → With the current implementation, this position cannot be liquidated, but in fact it should be liquidated, because liquidation will cost 105 tokens and only marginBalance above requiredMaintenanceMargin + liquidationFee should be treated as healthy.

In Synthetix's code, this check adds the liquidationFee.

Note: _liquidationMargin = requiredMaintenanceMargin

PerpsV2MarketBase.sol#L433-L440

function _canLiquidate(Position memory position, uint price) internal view returns (bool) {
// No liquidating empty positions.
if (position.size == 0) {
return false;
}
​
return _remainingLiquidatableMargin(position, price) <= _liquidationMargin(int(position.size), price);
}

PerpsV2MarketBase.sol#L391-L395

function _liquidationMargin(int positionSize, uint price) internal view returns (uint lMargin) {
uint liquidationBuffer =
_abs(positionSize).multiplyDecimal(price).multiplyDecimal(_liquidationBufferRatio(_marketKey()));
return liquidationBuffer.add(_liquidationFee(positionSize, price)).add(_keeperLiquidationFee());
}

Impact

Positions that, upon liquidation, will lose requiredMaintenanceMargin + liquidationFee, but their marginBalance is less than requiredMaintenanceMargin + liquidationFee and higher than requiredMaintenanceMargin, will not be liquidated.

Tools Used

Manual Review

Recommendations

When TradingAccount.isLiquidatable is called requiredMaintenanceMarginUsdX18 should add liquidationFeeUsdX18.

Updates

Lead Judging Commences

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

Liquidation doesn't take the liquidation fee in consideration inside the isLiquidatable check

Support

FAQs

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