Summary
In the liquidation process, all of a user's positions are closed, even if some of them are in profit. Ideally, only the positions that are in loss should be closed to prevent unnecessary liquidation of profitable positions. This approach would be more favorable for users, as it would minimize the impact on their accounts by preserving profitable positions and user has to pay order fee to reenter the market.
Vulnerability Details
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
ctx.marketId = ctx.activeMarketsIds[j].toUint128();
PerpMarket.Data storage perpMarket = PerpMarket.load(ctx.marketId);
Position.Data storage position = Position.load(ctx.tradingAccountId, ctx.marketId);
ctx.oldPositionSizeX18 = sd59x18(position.size);
ctx.liquidationSizeX18 = -ctx.oldPositionSizeX18;
ctx.markPriceX18 = perpMarket.getMarkPrice(ctx.liquidationSizeX18, perpMarket.getIndexPrice());
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, ctx.markPriceX18);
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);
position.clear();
tradingAccount.updateActiveMarkets(ctx.marketId, ctx.oldPositionSizeX18, SD59x18_ZERO);
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}
Here is the scenario, A user having a very profitable postion in ETHUSD market but loss in BTCUSD market but in liquidation closing of both positions happens.
USER_POS_SIZE_DELTA = 10e18
ETHUSD initial price = 1e21
ETHUSD updated price = 1.5e21
BTCUSD initial price = 1e23
BTCUSD updated price = 9e22
| Incident |
Marginal Balance |
Initial Margin |
Maintenance Margin |
Available Balance |
| Created Account |
1e23 |
0 |
0 |
1e23 |
| After opening ETHUSD Position |
9.998e22 |
1e20 |
5e19 |
9.988e22 |
| After ETHUSD price increment |
1.049e23 |
1.5e20 |
7.5e19 |
1.048e23 |
| After opening BTCUSD position |
1.041e23 |
1.015e22 |
5.075e21 |
9.403e22 |
| After BTCUSD price down |
4.187e21 |
9.15e21 |
4.575e21 |
-4.962e21 |
| Incident 6 |
5e21 |
1.5e20 |
7.5e19 |
4.85e21 |
Here, Incident6 is "only closing of loss postion in liquidation"
and user is not liquidatable after this.
PoC:
function test_CheckForOnlyCloseForLossPositionsInLiquidation() external {
uint256 USER_STARTING_BALANCE = 100_000e18;
int128 USER_POS_SIZE_DELTA = 10e18;
deal({ token: address(usdz), to: users.naruto.account, give: USER_STARTING_BALANCE });
changePrank({ msgSender: users.naruto.account });
uint128 tradingAccountId = createAccountAndDeposit(USER_STARTING_BALANCE, address(usdz));
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
openManualPosition(
ETH_USD_MARKET_ID, ETH_USD_STREAM_ID, MOCK_ETH_USD_PRICE, tradingAccountId, USER_POS_SIZE_DELTA
);
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
uint256 updatedPrice1 = MOCK_ETH_USD_PRICE + MOCK_ETH_USD_PRICE / 2;
updateMockPriceFeed(ETH_USD_MARKET_ID, updatedPrice1);
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
openManualPosition(
BTC_USD_MARKET_ID, BTC_USD_STREAM_ID, MOCK_BTC_USD_PRICE, tradingAccountId, USER_POS_SIZE_DELTA
);
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
uint256 updatedPrice = MOCK_BTC_USD_PRICE - MOCK_BTC_USD_PRICE / 10;
updateMockPriceFeed(BTC_USD_MARKET_ID, updatedPrice);
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
openManualPosition(BTC_USD_MARKET_ID, BTC_USD_STREAM_ID, updatedPrice, tradingAccountId, -USER_POS_SIZE_DELTA);
{
perpsEngine.getAccountMarginBreakdown({ tradingAccountId: tradingAccountId });
}
}
and there is another case- negative margins if BTCUSD prices drops to 8.888e22 then liquidation should close the Profit positions also.
Impact
unnecessary closing of profitable positions in liquidation while having positive margins also it may impact user when profitable market is bullish
Tools Used
Manual Review
Recommendations
Check for the margins are negative or positive after that verify if closing of loss Positions is only enough then clear loss positions in liquidation else then close all positions
