OrderBook

First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

Tokens Can Remain Locked in Expired Orders

Summary

The OrderBook contract does not automatically handle expired orders, which could lead to tokens being locked in the contract indefinitely if sellers forget to cancel their expired orders. This creates a potential asset freeze vulnerability.

Description

When a sell order is created, the seller's tokens are transferred to the contract and locked until the order is filled, cancelled, or expired. However, when an order expires, the tokens remain locked in the contract until the seller explicitly calls the cancelSellOrder function. This creates a scenario where sellers may forget to cancel their expired orders, resulting in their tokens being locked in the contract indefinitely. For large token amounts or in cases where many users forget to cancel their orders, significant value could be unnecessarily locked in the contract.

Step-by-step analysis

  1. A seller creates a sell order by calling createSellOrder, transferring their tokens to the contract.

  2. The order has a deadline (maximum 3 days from creation).

  3. If the order is not filled before the deadline, it expires.

  4. The contract correctly prevents buyers from purchasing expired orders.

  5. However, the tokens remain locked in the contract until the seller explicitly calls cancelSellOrder.

  6. If the seller forgets to cancel the expired order, their tokens remain locked indefinitely.

Severity classification

  • Impact: Medium - Tokens can be locked in the contract indefinitely, but the original owner can still recover them by cancelling the order.

  • Likelihood: High - It is common for users to forget to cancel expired orders, especially in a decentralised environment where users may not be actively monitoring all their transactions.

File name with issue

OrderBook.sol

Code with issue

function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
totalFees += protocolFee;
emit OrderFilled(_orderId, msg.sender, order.seller);
}

Recommendation

Implement a function to allow anyone to cancel expired orders, which will return the tokens to the original seller. This will prevent tokens from being locked indefinitely and will improve the user experience.

Code to fix the problem

function cancelExpiredOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderAlreadyInactive();
if (block.timestamp < order.deadlineTimestamp) revert("Order not yet expired");
// Mark as inactive
order.isActive = false;
// Return locked tokens to the seller
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
emit OrderCancelled(_orderId, order.seller);
}

The proposed fix allows anyone to cancel an order that has passed its deadline, returning the tokens to the original seller. This ensures that tokens do not remain locked in the contract indefinitely if sellers forget to cancel their expired orders.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Expired orders can cause backlog

By design only `seller` can call `cancelSellOrder()` on their `order`. But when an `order` expires, and the `seller` doesn't have access to the protocol, the expired `order `should be be able to be cancelled by an `admin`.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.