OrderBook

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

Lack of explicit expired order cancellation

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 1 year 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`.

Appeal created

deadmanxxxii Submitter
about 1 year ago
yeahchibyke Lead Judge about 1 year 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.

Give us feedback!