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

Offchain orders are not cancelled when Liquidation occurs in the market, hence if a user deposits more funds and prices immediately aligns trades are executed against user's wish

bigsam

Summary

In the current system, when an account is liquidated, only on-chain orders are cleared, leaving off-chain orders active. If a user deposits more funds after liquidation and the market prices align, these off-chain orders might get executed against the user’s intentions. This situation can open unwanted positions, contrary to the user’s wishes, particularly with Take Profit (TP) and Stop Loss (SL) orders.

Vulnerability Details

When an account is liquidated, the existing on-chain market orders are cleared, but the off-chain orders remain active. If the user deposits additional funds into their account and the market conditions match the criteria of these off-chain orders, they can be executed. This unintended execution can lead to undesired trades, such as opening short positions via TP orders and long positions via SL orders, against the user’s intentions.

### Key Points:

1. **Current Behavior**:

- On-chain market orders are cleared during liquidation.

- Off-chain orders remain active and are not invalidated.

2. **Issue**:

- After liquidation, if the user deposits more funds, off-chain orders might execute based on new market conditions.

- This can lead to unintended trades that do not align with the user’s current intentions.

3. **Desired Behavior**:

- Both on-chain and off-chain orders should be cleared during liquidation to prevent unintended trades.

/// @param accountsIds The list of accounts to liquidate
function liquidateAccounts(uint128[] calldata accountsIds) external {
---------------------------------------------------------------
​
@audit>> offchain order not cleared>>
// clear pending order for account being liquidated
@audit>> Only onchain orders are cleared>> MarketOrder.load(ctx.tradingAccountId).clear();
​
// copy active market ids for account being liquidated
ctx.activeMarketsIds = tradingAccount.activeMarketsIds.values();
-------------------------------------------------
}

Offchain order aligns with market condition on new deposit

​
/// @notice Fills pending, eligible offchain offchain orders targeting the given market id.
/// @dev If a trading account id owner transfers their account to another address, all offchain orders will be
/// considered cancelled.
/// @param marketId The perp market id.
/// @param offchainOrders The array of signed custom orders.
/// @param priceData The price data of custom orders.
function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{ -------------------------------------------
// account state updates start here
​
// increase the trading account nonce if the order's flag is true.
if (ctx.offchainOrder.shouldIncreaseNonce) {
unchecked {
tradingAccount.nonce++;
}
}
​
// mark the offchain order as filled.
// we store the struct hash to be marked as filled.
tradingAccount.hasOffchainOrderBeenFilled[ctx.structHash] = true;
​
// fill the offchain order.
_fillOrder(
ctx.offchainOrder.tradingAccountId,
marketId,
SettlementConfiguration.OFFCHAIN_ORDERS_CONFIGURATION_ID,
sd59x18(ctx.offchainOrder.sizeDelta),
ctx.fillPriceX18
);
}
}

The above order is successfully executed.

Impact

This vulnerability can lead to significant unintended trades, affecting the user’s positions. It is classified as a high-severity issue due to the direct involvement of user funds and the potential for executing trades against the user’s intentions.

Tools Used

- Manual Solidity code analysis

- Review of order management logic

Recommendations

**Clear Off-chain Orders During Liquidation**: Ensure that off-chain orders are also cleared when an account is liquidated. This can be achieved by invalidating the off-chain orders by bumping the nonce or similar mechanisms.

/// @param accountsIds The list of accounts to liquidate\
function liquidateAccounts(uint128\[] calldata accountsIds) external {
---------------------------------------------------------------
​
​
++ // bump the nonce value, which will invalidate all offchain orders signed with the previous nonce
++ unchecked {
++ tradingAccount.nonce++;
++ }
// 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();
-------------------------------------------------
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`LiquidationBranch.liquidateAccounts` should cancel off-chain orders of the liquidated account.

Support

FAQs

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