OrderBook

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

Lack of explicit expired order cancellation

Author Revealed upon completion

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

// Root cause in the codebase with @> marks to highlight the relevant section
Description (normal behavior):
Sellers can cancel their own orders, and orders automatically become invalid if expired. However, no function allows someone else to explicitly cancel expired orders.
Description (issue):
Expired orders remain marked as isActive, potentially locking seller tokens indefinitely and creating confusion for UIs and bots scanning on-chain states.
Risk:
Likelihood:
Medium.
Reason 1: Occurs after an order’s deadline has passed.
Reason 2: Sellers might lose access (e.g., key loss) and cannot recover tokens without a mechanism.
Impact:
Medium.
Tokens stuck indefinitely in contract.
Creates confusion and bloat in order tracking systems.

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Impact 1

  • Impact 2

Proof of Concept

if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // @> Only checked on buy, no forced cleanup
Explanation:
1. Seller creates order with deadline 3 days.
2. Order expires
3. Seller loses wallet keys.
4. Tokens remain locked in contract forever.

Recommended Mitigation

- remove this code
+ add this code
function cancelExpiredOrder(uint256 _orderId) external {
+ Order storage order = orders[_orderId];
+ if (order.seller == address(0)) revert OrderNotFound();
+ if (!order.isActive) revert OrderAlreadyInactive();
+ if (block.timestamp < order.deadlineTimestamp) revert OrderNotExpired();
+ order.isActive = false;
+ IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
+ emit OrderCancelled(_orderId, order.seller);
+ }
Explanation:
🟢 Function purpose
✅ Why we need it
Right now, expired orders become ineligible to buy, but the contract doesn’t provide a way to explicitly cancel them and unlock the seller's tokens if the seller is unavailable.
This new function allows anyone to forcefully cancel an expired order and return tokens to the seller.
---
🔬 Line-by-line explanation
function cancelExpiredOrder(uint256 _orderId) external {
Function signature.
It’s marked external so it can be called by anyone (not just the seller or owner).
---
Order storage order = orders[_orderId];
Retrieves the Order struct from the orders mapping using the given _orderId.
We use storage because we want to modify the actual order in storage, not just read it.
---
if (order.seller == address(0)) revert OrderNotFound();
Checks if the order exists.
In your design, a non-existent order has seller == address(0).
If it doesn’t exist, revert with OrderNotFound custom error.
---
if (!order.isActive) revert OrderAlreadyInactive();
Checks if the order is already inactive (either filled or previously cancelled).
If so, we don’t need to cancel it again, so it reverts with OrderAlreadyInactive.
---
if (block.timestamp < order.deadlineTimestamp) revert OrderNotExpired();
Checks if the order’s deadline has already passed.
If it has not expired yet (current block timestamp is before the deadline), we do not allow anyone to cancel it yet.
If it’s not expired, it reverts with a new error OrderNotExpired (you would define this custom error).
---
order.isActive = false;
Marks the order as inactive to prevent future buys or further cancellations.
This step is always done before external calls (like transfers) to avoid reentrancy risks.
---
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
Transfers the tokens held in escrow back to the seller.
Uses safeTransfer from SafeERC20 to handle possible non-standard ERC20 return values properly.
---
emit OrderCancelled(_orderId, order.seller);
Emits an event to record on-chain that the order was canceled.
This helps frontends or indexers know to update their UI and display that the order is no longer active.
---
💬 Summary of purpose
✅ Allows anyone to "clean up" expired orders.
✅ Returns tokens to sellers automatically.
✅ Prevents tokens from being stuck forever if the seller can’t or won’t cancel.
✅ Improves order book hygiene and reduces UI confusion.
---
🟡 New error definition needed
You’ll also want to define this error to match your style:
error OrderNotExpired();
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 5 hours 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.