Submission Details
Severity:
Inability to AJust/open a Position that is not liquidatable/below required margin because of a wrong Validation
Summary
The liquidation fee was included in the settlement fee for validating a close order hence the margin validation becomes inflated and prevents users from adjusting their positions even though their position is not presently liquidatable.
DOS to a time sensitive function causing users to lose their collateral.
Vulnerability Details
Addition of the liquidation fee to settlement fees while trying to validate a closing position will cause Healthy positions close to liquidation to revert and never be closed risking liquidation and asset loss.
function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
internal
virtual
{
// working data
FillOrderContext memory ctx;
// fetch storage slot for perps engine configuration
PerpsEngineConfiguration.Data storage perpsEngineConfiguration = PerpsEngineConfiguration.load();
// fetch storage slot for perp market's settlement config
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, settlementConfigurationId);
// determine whether position is being increased or not
ctx.isNotionalValueIncreasing =
Position.isNotionalValueIncreasing(tradingAccountId, marketId, sizeDeltaX18.intoInt256().toInt128());
// both markets and settlement can be disabled, however when this happens we want to:
// 1) allow open positions not subject to liquidation to decrease their size or close
// 2) prevent new positions from being opened & existing positions being increased
//
// the idea is to prevent a state where traders have open positions but are unable
// to reduce size or close even though they can still be liquidated; such a state
// would severly disadvantage traders
if (ctx.isNotionalValueIncreasing) {
// both checks revert if disabled
perpsEngineConfiguration.checkMarketIsEnabled(marketId);
settlementConfiguration.checkIsSettlementEnabled();
}
// load existing trading account; reverts for non-existent account
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
// enforce minimum trade size
perpMarket.checkTradeSize(sizeDeltaX18);
// get funding rates for this perp market
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, fillPriceX18);
// update funding rates
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);
// calculate order & settlement fees
ctx.orderFeeUsdX18 = perpMarket.getOrderFeeUsd(sizeDeltaX18, fillPriceX18);
ctx.settlementFeeUsdX18 = ud60x18(uint256(settlementConfiguration.fee));
// fetch storage slot for account's potential existing position in this market
Position.Data storage oldPosition = Position.load(tradingAccountId, marketId);
// int256 -> SD59x18
ctx.oldPositionSizeX18 = sd59x18(oldPosition.size);
{
// calculate required initial & maintenance margin for this trade
// and account's unrealized PNL
(
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
) = tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, sizeDeltaX18);
// check maintenance margin if:
// 1) position is not increasing AND
// 2) existing position is being decreased in size
//
// when a position is under the higher initial margin requirement but over the
// lower maintenance margin requirement, we want to allow the trader to decrease
// their losing position size before they become subject to liquidation
//
// but if the trader is opening a new position or increasing the size
// of their existing position we want to ensure they satisfy the higher
// initial margin requirement
ctx.shouldUseMaintenanceMargin = !ctx.isNotionalValueIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
// reverts if the trader can't satisfy the appropriate margin requirement
tradingAccount.validateMarginRequirement(
ctx.requiredMarginUsdX18,
tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18).add(
@audit>> ud60x18(perpsEngineConfiguration.liquidationFeeUsdX18) //BUG @audit what is liquidation fee doing here
)
);
}
The validation uses an inflated total fee causing a healthy position to revert
function validateMarginRequirement(
Data storage self,
UD60x18 requiredMarginUsdX18,
SD59x18 marginBalanceUsdX18,
UD60x18 totalFeesUsdX18
)
internal
view
{
@audit>> revert>> if (requiredMarginUsdX18.add(totalFeesUsdX18).intoSD59x18().gt(marginBalanceUsdX18)) { //Note
revert Errors.InsufficientMargin(
self.id,
marginBalanceUsdX18.intoInt256(),
requiredMarginUsdX18.intoUint256(),
totalFeesUsdX18.intoUint256()
);
}
}
When actually when we want to settle a position we do not deduct liquidation fee hence it should not be part of this verification.
// pay order/settlement fees and deduct collateral
// if trader's old position had negative pnl
tradingAccount.deductAccountMargin(
TradingAccount.DeductAccountMarginParams({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: perpsEngineConfiguration.marginCollateralRecipient,
orderFeeRecipient: perpsEngineConfiguration.orderFeeRecipient,
settlementFeeRecipient: perpsEngineConfiguration.settlementFeeRecipient
}),
pnlUsdX18: ctx.pnlUsdX18.lt(SD59x18_ZERO) ? ctx.pnlUsdX18.abs().intoUD60x18() : UD60x18_ZERO,
@audit>> orderFeeUsdX18: ctx.orderFeeUsdX18,
@audit>> settlementFeeUsdX18: ctx.settlementFeeUsdX18,
marketIds: ctx.marketIds,
accountPositionsNotionalValueX18: new UD60x18[](1) // when we have only one market id, this property
// isn't used, so we can pass an empty array
})
);
The liquidation fee is used to check if a position is liquidatable and should not prevent settlement
// prevent liquidatable accounts from trading
if (
TradingAccount.isLiquidatable(
requiredMaintenanceMarginUsdX18,
marginBalanceUsdX18,
ud60x18(perpsEngineConfiguration.liquidationFeeUsdX18)
)
) {
revert Errors.AccountIsLiquidatable(tradingAccountId);
}
see below
/// @notice Checks if the account is liquidatable.
/// @param requiredMaintenanceMarginUsdX18 The required maintenance margin in USD.
/// @param marginBalanceUsdX18 The account's margin balance in USD.
/// @param liquidationFeeUsdX18 The liquidation fee in USD.
function isLiquidatable(
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 marginBalanceUsdX18,
UD60x18 liquidationFeeUsdX18
)
internal
pure
returns (bool)
{
@audit >> return requiredMaintenanceMarginUsdX18.add(liquidationFeeUsdX18).intoSD59x18().gt(marginBalanceUsdX18);
}
Impact
Adding the liquidation fee to a position we are adjusting close to the liquidation point will revert a healthy position and DOS a healthy position adjustment.
This will also DOS a user who has the enough initial margin to open a position as the transaction will revert because the fee to be paid used in the check is inflated, more than the user actually pays.
Tools Used
Manual review
Recommendations
Remove the liquidation fee from the validation and check if a position is either liquidated or if the trade been adjusted is healthy with the order fee plus the settlement fee covered.
Updates
Appeal created
bigsam
6 months ago
bigsam
6 months ago
bigsam
6 months ago
inallhonesty
6 months ago
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821 USDC / LOC
66,500
3,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.