Summary
Users lose their funds when the cancelOrderFarFromOracle() is called.
Vulnerability Details
When the orderId limit 65000 is reached any user would have the ability to call the function cancelOrderFarFromOracle
function cancelOrderFarFromOracle(
address asset,
O orderType,
uint16 lastOrderId,
uint16 numOrdersToCancel
) external onlyValidAsset(asset) nonReentrant {
if (s.asset[asset].orderId < 65000) {
revert Errors.OrderIdCountTooLow();
}
if (numOrdersToCancel > 1000) {
revert Errors.CannotCancelMoreThan1000Orders();
}
if (msg.sender == LibDiamond.diamondStorage().contractOwner) {
if (
orderType == O.LimitBid
&& s.bids[asset][lastOrderId].nextId == Constants.TAIL
) {
s.bids.cancelManyOrders(asset, lastOrderId, numOrdersToCancel);
} else if (
orderType == O.LimitAsk
&& s.asks[asset][lastOrderId].nextId == Constants.TAIL
) {
s.asks.cancelManyOrders(asset, lastOrderId, numOrdersToCancel);
} else if (
orderType == O.LimitShort
&& s.shorts[asset][lastOrderId].nextId == Constants.TAIL
) {
s.shorts.cancelManyOrders(asset, lastOrderId, numOrdersToCancel);
} else {
revert Errors.NotLastOrder();
}
} else {
if (
orderType == O.LimitBid
&& s.bids[asset][lastOrderId].nextId == Constants.TAIL
) {
s.bids.cancelOrder(asset, lastOrderId);
} else if (
orderType == O.LimitAsk
&& s.asks[asset][lastOrderId].nextId == Constants.TAIL
) {
s.asks.cancelOrder(asset, lastOrderId);
} else if (
orderType == O.LimitShort
&& s.shorts[asset][lastOrderId].nextId == Constants.TAIL
) {
s.shorts.cancelOrder(asset, lastOrderId);
} else {
revert Errors.NotLastOrder();
}
}
}
However, this function calls the cancelOrder() function directly which only changes the order Id and does not refund user tokens.
function cancelOrder(
mapping(address => mapping(uint16 => STypes.Order)) storage orders,
address asset,
uint16 id
) internal {
uint16 prevHEAD = orders[asset][Constants.HEAD].prevId;
orders[asset][orders[asset][id].nextId].prevId = orders[asset][id].prevId;
orders[asset][orders[asset][id].prevId].nextId = orders[asset][id].nextId;
emit Events.CancelOrder(asset, id, orders[asset][id].orderType);
_reuseOrderIds(orders, asset, id, prevHEAD, O.Cancelled);
}
In addition, when the cancelOrder() function is called the orderType is changed to O.Cancelled. this means that when the user tries to cancel his order again using the cancelBid() or cancelAsk() function, the call will revert due to inactiveOrder. This will lead to an eternal loss of funds.
Impact
Users will lose their funds
Tools Used
Manual
Recommendations
according to what the order type is and what assets are used try to call either cancelAsk() or cancelBid() without a direct call to cancelOrder()
