Summary

The CancelOrderFarFromOracle function poses a significant risk of funds loss, as it cancels orders without returning assets or ETH to their owners.

Vulnerability Details

The vulnerability resides in the library functions cancelOrder and cancelManyOrders, which are employed by CancelOrderFarFromOracle(). In the current implementation, this function cancels orders by removing them from the order book, and users who own these orders cannot cancel them again using the correct functions (such as cancelShort, cancelAsk, or cancelBid).

Consequently, the owners of the cancelled orders lose their assets or ETH, depending on the order type, as the assets are not returned to the Escrow.

The following lines are missing in the execution of the problematic function, as only cancelOrder is invoked:

• cancelShort

• cancelAsk

• cancelBid

Simple POC

A test case demonstrates that the ETH of a short order is not returned when cancelled using cancelOrderFarFromOracle(). This test is included in the CancelOrder.t.sol file.

Impact

Owners of orders cancelled by this function stand to lose ERC assets or ETH, depending on the order type. While the likelihood of this vulnerability occurring is relatively low/medium, as the order book must be fulfilled to enable the cancellation of the last order by any user, but it's important to note that when the DAO calls this function, funds loss can still occur.

As a result, I classify the severity of this vulnerability as Medium/High.

Tools Used

Manual code review

Recommendations

To address this vulnerability, ensure that when orders are cancelled via this function, assets or ETH are returned to their respective owners. This can be achieved by adding the necessary logic to cancelOrderFarFromOracle from following lines.

• cancelShort

• cancelAsk

• cancelBid