DittoETH

Summary

Reverts entire transaction when incomingAsk.ercAmount > highestBid.ercAmount && startingId == Constants.TAIL && orderHint.lenght == 0 when it should rather end the transaction.

Vulnerability Details

There a couple of instances when a user might not supply a hintId, since it is not required all the time when calling createAsks
as one can supply an empty array.

POC:

1. Bob Creates a market ask order with a price of $1500 and amount 200.

2. While he creates the request he sees that on the order book there are two orders that matches his market order, in the sense that they are both either greater than or equal to Bob's order amount.

3. Bob proceeds to make the request, supplying orderHintId as [] (empty array),

4. Bob request remains in the mempool for about 10 minutes due to low gas and by the time it is finally executed the two intended matches have been matched/closed.

5. Now the current state of Bob's request against the protocol is incomingAsk.ercAmount > highestBid.ercAmount && startingId == Constants.TAIL && orderHint.lenght == 0 .

6. Since Bob wants 200 units of the requested asset and the available is only 150, and the bid table is empty after matching the 150 asset amount.

7. The Contract then attempts to add Bobs left over 50 asset amount at Price to the ask order book , but Bob did not supply an orderhintArray at request, because of this the entire transaction reverts.

Impact

Revert transaction when it is meant to end transaction.

Tools Used

Manual

Recommendations

It should rather just end the execution when it sees that orderHintArray.lenght == 0 as opposed to reverting the entire transaction. that way the already fulfilled order stands.

if (incomingAsk.ercAmount.mul(incomingAsk.price) >= minAskEth) {
    if(orderHintArray.lenght != 0){
    	addSellOrder(incomingAsk, asset, orderHintArray);
    }
}