Submission Details
Severity:
Missing PnL Minting Before Margin Deduction in liquidateAccounts Function
Summary
The liquidateAccounts function is responsible for liquidating undercollateralized accounts by closing their open positions, deducting the required margin, and transferring liquidation fees. However, if the liquidated user has a positive PnL (profit and loss), the function does not mint the positive PnL to the liquidated account before calling deductAccountMargin.
Vulnerability Details
function liquidateAccounts(uint128[] calldata accountsIds) external {
// return if no input accounts to process
if (accountsIds.length == 0) return;
// fetch storage slot for perps engine configuration
PerpsEngineConfiguration.Data storage perpsEngineConfiguration = PerpsEngineConfiguration.load();
// only authorized liquidators are able to liquidate
if (!perpsEngineConfiguration.isLiquidatorEnabled[msg.sender]) {
revert Errors.LiquidatorNotRegistered(msg.sender);
}
// working data
LiquidationContext memory ctx;
// load liquidation fee from perps engine config; will be passed in as `settlementFeeUsdX18`
// to `TradingAccount::deductAccountMargin`. The user being liquidated has to pay
// this liquidation fee as a "settlement fee"
ctx.liquidationFeeUsdX18 = ud60x18(perpsEngineConfiguration.liquidationFeeUsdX18);
// iterate over every account being liquidated; intentionally not caching
// length as reading from calldata is faster
for (uint256 i; i < accountsIds.length; i++) {
// store current accountId being liquidated in working data
ctx.tradingAccountId = accountsIds[i];
// sanity check for non-sensical accountId; should never be true
if (ctx.tradingAccountId == 0) continue;
// load account's leaf (data + functions)
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(ctx.tradingAccountId);
// get account's required maintenance margin & unrealized PNL
(, ctx.requiredMaintenanceMarginUsdX18, ctx.accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
// get then save margin balance into working data
ctx.marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(ctx.accountTotalUnrealizedPnlUsdX18);
// if account is not liquidatable, skip to next account
// account is liquidatable if requiredMaintenanceMarginUsdX18 > ctx.marginBalanceUsdX18
if (
!TradingAccount.isLiquidatable(
ctx.requiredMaintenanceMarginUsdX18, ctx.marginBalanceUsdX18, ctx.liquidationFeeUsdX18
)
) {
continue;
}
// clear pending order for account being liquidated
MarketOrder.load(ctx.tradingAccountId).clear();
// copy active market ids for account being liquidated
ctx.activeMarketsIds = tradingAccount.activeMarketsIds.values();
// instatiate the array of positions in usd value
ctx.accountPositionsNotionalValueX18 = new UD60x18[](ctx.activeMarketsIds.length);
// iterate over memory copy of active market ids
// intentionally not caching length as expected size < 3 in most cases
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
// load current active market id into working data
ctx.marketId = ctx.activeMarketsIds[j].toUint128();
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(ctx.marketId);
// load position data for user being liquidated in this market
Position.Data storage position = Position.load(ctx.tradingAccountId, ctx.marketId);
// save open position size
ctx.oldPositionSizeX18 = sd59x18(position.size);
// save inverted sign of open position size to prepare for closing the position
ctx.liquidationSizeX18 = -ctx.oldPositionSizeX18;
// calculate price impact of open position being closed
ctx.markPriceX18 = perpMarket.getMarkPrice(ctx.liquidationSizeX18, perpMarket.getIndexPrice());
// calculate notional value of the position being liquidated and push it to the array
ctx.accountPositionsNotionalValueX18[j] =
ctx.oldPositionSizeX18.abs().intoUD60x18().mul(ctx.markPriceX18);
// get current funding rates
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, ctx.markPriceX18);
// update funding rates for this perpetual market
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);
// reset the position
position.clear();
// update account's active markets; this calls EnumerableSet::remove which
// is why we are iterating over a memory copy of the trader's active markets
tradingAccount.updateActiveMarkets(ctx.marketId, ctx.oldPositionSizeX18, SD59x18_ZERO);
// we don't check skew during liquidations to protect from DoS
(ctx.newOpenInterestX18, ctx.newSkewX18) = perpMarket.checkOpenInterestLimits(
ctx.liquidationSizeX18, ctx.oldPositionSizeX18, SD59x18_ZERO, false
);
// update perp market's open interest and skew; we don't enforce ipen
// interest and skew caps during liquidations as:
// 1) open interest and skew are both decreased by liquidations
// 2) we don't want liquidation to be DoS'd in case somehow those cap
// checks would fail
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}
// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin(
TradingAccount.DeductAccountMarginParams({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: perpsEngineConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: perpsEngineConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: ctx.accountTotalUnrealizedPnlUsdX18.abs().intoUD60x18().add(
ctx.requiredMaintenanceMarginUsdX18
),
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18,
marketIds: ctx.activeMarketsIds,
accountPositionsNotionalValueX18: ctx.accountPositionsNotionalValueX18
})
);
emit LogLiquidateAccount(
msg.sender,
ctx.tradingAccountId,
ctx.activeMarketsIds.length,
ctx.requiredMaintenanceMarginUsdX18.intoUint256(),
ctx.marginBalanceUsdX18.intoInt256(),
ctx.liquidatedCollateralUsdX18.intoUint256(),
ctx.liquidationFeeUsdX18.intoUint128()
);
}
}
If a liquidated account has unrealized profits (positive PnL), these profits should be credited to the account before deducting the margin. The function correctly calculates ctx.accountTotalUnrealizedPnlUsdX18. However, if the PnL is positive, it should be minted to the account before the deductAccountMargin call. Instead, the function immediately deducts margin, which can result in excessive deductions and unfair liquidation penalties.
Impact
The liquidated trader loses rightful profits because their PnL was not considered before margin deduction.
Tools Used
Manual Review
Recommendations
Mint Positive PnL Before Margin Deduction
Updates
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.