Zaros Part 1

Vulnerability Details

There are multiple ways to create an offChainOrder to ensure that when the keeper calls fillOffchainOrders it will revert

Here are some of the ways, there are a lot more but are not relevant because they all have the same fix

1. Pass a 0 sizeDelta, will revert here

2. pass a non-existent account, will revert here

3. transferring the accountNFT to an alt, will revert here

This is an issue for 2 reasons:

1. The keeper's Tx reverts, so they have been griefed

2. fillOffchainOrders takes in a list of offchainOrders, if just one of them reverts then all the other orders will not fill. Therefore an attacker can easily spam reverting orders to delay honest user's orders from filling

When an incompatible off chain order is processed it should skip filling it and move on to the next order in the array rather than reverting the whole tx

Impact

keeper loses funds

The other users that would have been filled are having their orders delayed because of one attacker

Tools Used

Manual Review

Recommendations

wrap each iteration of the loop in a try catch block, so that if one reverts it moves on to the next

Or use continue instead of revert