DittoETH

Summary

in LibOrders.sol contract In CancelManyOrders function it can't cancel the values more than 255 . because in the for loop i value is uint8 only.
numOrdersToCancel is more than the 255 then is leads to unintended behavior.

Vulnerability Details

function cancelManyOrders(
mapping(address => mapping(uint16 => STypes.Order)) storage orders,
address asset,
uint16 lastOrderId,
uint16 numOrdersToCancel
) internal {
uint16 prevId;
uint16 currentId = lastOrderId;
for (uint8 i; i < numOrdersToCancel;) {
prevId = orders[asset][currentId].prevId;
LibOrders.cancelOrder(orders, asset, currentId);
currentId = prevId;
unchecked {
++i;
}
}
}
in the above one the for loop i range is uint8 and numOrdersToCancel range is uint16. so when the value is more than uint8 then it comes to the inital number.
suppose take a simple example
numOrdersToCancel this value is 300 then the uint8 range is 0-255 it leads to overflow.
uint8 0-255
so 255+1 =0 and 255+2=1 so it con't cancel the all orders more than 255.

Type Mismatch: numOrdersToCancel is of type uint16, which means it can hold values from 0 to 65,535. On the other hand, i is of type uint8, which can only hold values from 0 to 255.

Impact

not many orders to cancel it leads to unintended behavior

Tools Used

manual

Recommendations

function cancelManyOrders(
mapping(address => mapping(uint16 => STypes.Order)) storage orders,
address asset,
uint16 lastOrderId,
uint16 numOrdersToCancel
) internal {
uint16 prevId;
uint16 currentId = lastOrderId;
for (uint16 i; i < numOrdersToCancel;) {
prevId = orders[asset][currentId].prevId;
LibOrders.cancelOrder(orders, asset, currentId);
currentId = prevId;
unchecked {
++i;
}
}
}
in the above code it is correct it con't lead to overflow. and all numOrdersToCancel can Execute .