Zaros Part 1

Summary

Off-chain keepers calls to fillOffchainOrders() could be DOSed on purpose or revert more times than necessary, thus preventing the execution of possibly time-sensitive orders and bringing losses to users.

Vulnerability Details

Off-chain keepers call fillOffchainOrders() to execute multiple off-chain orders for a specific market that are eligible for execution.

The function loops through all off-chain order struct argument details and executes them one by one.

There are many reasons however, an off-chain order would revert and would stop the execution of all off-chain orders it is batched with.

Such reasons could include:

• a user canceling his off-chain orders by increasing his nonce

• a user transferring his trading account NFT to another EOA making the initial signatures for his off-chain orders invalid

• reasons connected with any of the if statements inside the off-chain orders loop.

This would cause a delay in the execution of orders that could be crucial for users such as stop-loss, take-profit or other orders.

In times of high volatility time is of essence and a difference by a couple of minutes could have huge impact on the portfolio of users.

Furthermore, a malicious user could track the price changes provided by Chainlink and in times where a market experiences high volatility (big drop or big spike in price) he could predict if there would be many stop-loss or take-profit orders about to be executed by a keeper and try to delay them (notice without front-running - without watching transactions in a mempool since this is not possible on Arbitrum) by creating many off-chain orders, signing them and then either call cancelAllOffchainOrders() or simply transfer the trading account NFT to another EOA that he owns.

If some of his off-chain orders get included in a batch of other valid off-chain orders, he will be able to successfully stop their execution.

Impact

Delaying the execution of possibly time sensitive offchain orders and bringing potential losses to users.

Tools Used

Manual Review

Recommended Mitigation

Without having access to the off-chain keeper logic that includes/discards off-chain orders in a batch, it is not possible to know the severity of the DOS (if the fillOffchainOrders() call would revert all the time or it will revert far less times).

From the smart contract perspective the issue is easily fixable by skipping the offchain order loop iteration with continue instead of using revert. This way if an order is invalid for whatever reason the loop could simply continue executing the rest of the orders instead of stopping the whole execution.