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

SEV 17: Protocol does not account full collateral of trader as liquidatable amount and that losses can be greater than the maintenance margin

Severity: High

Summary

The protocol deducts maintenance margin value of the account from the collateral upon liquidation. The loss can be greater than maintenance margin. As a result, the protocol incures debt even when the debt can be covered with trader's collateral.

Vulnerability Details

Code: LiquidationBranch::liquidateAccounts; LiquidationBranch.sol#L138-L161

The account is considered liquidatable if the margin balance is less than the required maintenance margin. The protocol deducts the requiredMaintenanceMargin to cover the debt. However, it is possible that debt is more than the requiredMaintenanceMargin resulting in bad debt for the protocol.

POC:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
import { UD60x18, ud60x18, convert as ud60x18Convert } from "@prb-math/UD60x18.sol";
import { SD59x18, sd59x18 } from "@prb-math/SD59x18.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";
contract LiquidationIncursProtocolDebt is Test {
function test_liquidation() external {
UD60x18 btcFillPrice = ud60x18Convert(68818);
UD60x18 ethPrice = ud60x18Convert(3312);
// @s3v3ru5: MMR = IMR/2, Loss can still be greater than maintenance margin
uint initialMarginRate = 0.01e18;
uint maintenanceMarginRate = 0.005e18;
int256 positionSize = 50e18;
uint256 weethLTV = 0.7e18; // 70%
uint256 userEthDeposit = 20e18;
// after LTV
uint256 userEthCollateral = ud60x18(userEthDeposit).mul(ud60x18(weethLTV)).intoUint256();
UD60x18 notionalValue = sd59x18(positionSize).abs().intoUD60x18().mul(btcFillPrice);
UD60x18 initialMargin = ud60x18(initialMarginRate).mul(notionalValue);
// margin balance > initialMargin
assert(ud60x18(userEthCollateral).mul(ethPrice).gt(initialMargin));
// @s3v3ru5: Loss can be greater than margin balance even if collateral value stays the same
UD60x18 ethNewPrice = ethPrice;
// @s3v3ru5: Loss at fill price `68232` becomes `-29300` and maintenance margin is `17057`. The account is not
// liquidatable because of margin balance of `46368 - 29300 = 17068`.
// At fill price: 68231, maintenance margin = 17057.75, Loss = -29350, margin balance = `46368 - 29350 = 17018`.
// Assumption is the account is immediately liquidated
UD60x18 btcNewFillPrice = ud60x18Convert(68231);
SD59x18 priceShift = btcNewFillPrice.intoSD59x18().sub(btcFillPrice.intoSD59x18());
SD59x18 unrealizedPnlUsdX18 = sd59x18(positionSize).mul(priceShift);
notionalValue = sd59x18(positionSize).abs().intoUD60x18().mul(btcNewFillPrice);
UD60x18 maintenanceMargin = ud60x18(maintenanceMarginRate).mul(notionalValue);
UD60x18 marginBalance = ud60x18(userEthCollateral).mul(ethNewPrice);
assert(maintenanceMargin.gt(marginBalance.intoSD59x18().add(unrealizedPnlUsdX18).intoUD60x18()));
uint decimals = 18;
emit log_named_decimal_int("Loss (USD)", unrealizedPnlUsdX18.intoInt256(), 18);
emit log_named_decimal_uint("Deducted Amount (maintenance margin) (USD)", maintenanceMargin.intoUint256(), 18);
emit log_named_decimal_uint("Trader collateral value (USD)", ud60x18(userEthDeposit).mul(ethNewPrice).intoUint256(), decimals);
emit log_named_decimal_int("Protocol debt (USD)", maintenanceMargin.intoSD59x18().sub(unrealizedPnlUsdX18.abs()).intoInt256(), decimals);
}
}
> forge test --match-contract LiquidationIncursProtocolDebt -vv --via-ir
Logs:
Loss (USD): -29350.000000000000000000
Deducted Amount (maintenance margin) (USD): 17057.750000000000000000
Trader collateral value (USD): 66240.000000000000000000
Protocol debt (USD): -12292.250000000000000000

Impact

Protocol incurs unnecessary bad debt upon liquidation even when it can be covered using trader's collateral.

Tools Used

Manual Review

Recommendations

Use the loss amount (-accountTotalUnrealizedPnlUsdX18) for the pnlUsdX18 parameter in the call to TradingAccount::deductAccountMargin.

change LiquidationBranch.sol#L158 to

pnlUsdX18: unary(accountTotalUnrealizedPnlUsdX18).intoUD60x18(),
Updates

Lead Judging Commences

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

deductAccountMargin() treats `required maintenance margin` as the `unrealized PnL` because it uses `requiredMaintenanceMarginUsdX18` in place of `pnlUsdX18`

Support

FAQs

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