Zaros Part 1

Filling pending offchain for a specific market Id is carry through SettlementBranch::fillOffchainOrder, within this function an array of offchain orders is filled one by one using a for loop, the issue resides in the fact that said for loop has checks that revert, such us sizeDelta == 0 and marketId != ctx.offchainOrder.marketId, and if one of those conditions is not met, the function will revert and no order will be filled.

Vulnerability Details

As follows, I've listed all checks that will make the for loop revert:

• ctx.offchainOrder.sizeDelta == 0

• marketId != ctx.offchainOrder.marketId

• offchainOrder.nonce != tradingAccount.nonce

• tradingAccount.hasOffchainOrderBeenFilled[ctx.structHash]

• ctx.signer != tradingAccount.owner

• globalConfiguration.checkMarketIsEnabled(marketId)

• settlementConfiguration.checkIsSettlementEnabled()

The last two happen during SettlementBranch::_fillOrder.

Impact

If said situation does occur, the whole process of offchain order filling will be disrupted and not even valid offchain Orders will be fulfilled, and since said orders have a targetPrice, this could be a missing chance, which directly affects the traders.

Tools Used

Manual review

Recommendations

Insted of revert use continue to skip to next order so the process doesn't get stuck. Additionally you may add a function to check all those parameters beforehand, placing the orders in two arrays, one for the orders that passes them and can be filled, and another for the ones that do not pass and must me cancel or rejected.