Zaros Part 1

Summary

A user can get liquidated even if an order has been created that would have fixed its position, if the liquidationKeeper transaction is executed before the MarketOrderKeeper tx.

Vulnerability Details

Let's say that a user has a position close to the maintenance margin, so he decide to create a market order to reduce the size of its position.

1. The transaction is sent, and now the order must be filled by the MarketOrderKeeper through SettlementBranch::fillMarketOrder.

2. But before the order is filled, a transaction updated the price feed of the market where the position is, causing the position to now be liquidatable.

3. A LiquidationKeeper detect that position, and calls LiquidationBranch::liquidateAccounts through LiquidationKeeper::performUpkeep

A race condition can occur where performUpkeep->liquidateAccounts Tx is executed before the fillMarketOrder Tx.
The user gets liquidated even though he acted before its position was below the maintenance margin

This could have been prevented if liquidateAccounts had simulated the order to check if this would have made the position healthy.
After simulating it, the order could then be executed and the liquidation canceled for this user as their.

Impact

because of a race condition between the order filling, the price update of the asset, and a liquidation, a user can get unfairly liquidated.

Tools Used

Manual review

Recommendations

Rather than clearing the pending market order before liquidating, simulate the order and if it make the position healthy (above maintenance margin), fill the order.