DittoETH

Summary

Protocol documentation says that DAO is able to cancel up to 1,000 orders when order count is above 65,000. However, because of the faulty for loop it is impossible to cancel more than 255 orders.

Vulnerability details

orderId is implemented in protocol to index orders in orderbook. In the protocol documentation it is written that it can handle above 65,000 orders because of reusable orderIds. When there are more than 65,500 orders DAO can cancel up to 1,000 orders. Here are the code blocks from cancelOrderFarFromOracle function which allows DAO to cancel orders. It also allows user to cancel one order.

It makes sure that there are more than 65,000 orders.

This ensures that DAO can't cancel more than 1,000 orders.

Later cancelOrderFarFromOracle checks if msg.sender == LibDiamond.diamondStorage().contractOwner and based on the boolean value (true or false) of this statement it allows to cancel the desired amount of orders.

The problem occurs in cancelManyOrders (LibOrders.sol) which is called on the mapping of orders of specified earlier orderType.

This function receives parameters:

• mapping of orders to cancel

• address of asset (market that will be impacted)

• last order id

• number of orders to cancel

When we look at the implementation of this function we can see that uint8 was used as a variable for the iteration in the for loop. uint8 i maximum value is 255. As we can see in the for loop there is unchecked statement which allows uint underflow / overflow.

So when we try to add 1 to 255 (255 + 1) solidity would automaticly revert due to uint overflow but when we use unchecked solidity allows us to do this operation and the result of this will be 0.

When DAO would like to cancel more than 255 orders it would result in infinite loop since:

• the for loop will iterate when i < numOrdersToCancel

• the vaule of i will always be less than 256 because it can't get bigger than that due to overflow

i = 255 and i < 256 unchecked {++i;}
Next iteration
i = 0 and i < 256 unchecked {++i;}

PoC / Scenario

I created pretty simple PoC in Remix.

To see that this function can't handle more than 255 orders cancelations run this function with input parameter (amountOfItertions) equal to 256 or above.

Further explenation

After DAO tries to cancel more than 255 orders the infinite loop will be created which will terminate the transaction.

The transaction will fail because of gas consumption. For loop will run as many times as it can with provided gas. Since it will try to run infinitely it will run out of gas.

Impact

Protocol documentation states that DAO is able to cancel 1,000 orders. Since it is not possible with the current implementation of the code this issue disrupts protocols functionality. The implemented code can't handle desired functionality.

Tools used

VScode, Manual Review, Remix

Recommendations

To solve this problem change uint8 i to uint16 or any higher uint that can handle the desired amount of iterations.