OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View all submissions
Previous Next
Submission Details
Severity: low
Valid

Expired Orders Not Cancellable by Anyone (Design Flaw)

Author Revealed upon completion

Root + Impact

Description

  • Normally, once an order has expired (past its deadline), it should be possible to remove the order and return tokens to the seller, freeing up storage and preventing locked funds.

  • In the current implementation, only the original seller can cancel their expired order. If the seller becomes inactive or loses access, the expired order cannot be cancelled by anyone else, resulting in tokens being locked in the contract and permanent storage bloat.

function cancelSellOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
​
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (order.seller != msg.sender) revert NotOrderSeller(); // @> Only the seller can cancel, even if expired
if (!order.isActive) revert OrderAlreadyInactive(); // Already inactive (filled or cancelled)
​
// 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);
}

Risk

Likelihood:

  • Sellers frequently lose private keys, abandon accounts, or become inactive over time, especially on public DeFi platforms.

  • As time passes and the number of users grows, the contract will accumulate more expired orders that cannot be cancelled by anyone else.

Impact:

  • User funds may become permanently locked in expired orders, reducing trust in the protocol.

  • Contract storage will bloat with unremovable expired orders, increasing gas costs and potentially hindering future upgrades or migrations.

Proof of Concept

// Seller creates a sell order
orderBook.createSellOrder(...); // Seller's address
​
// Seller loses access to their account (private key lost)
​
// Order expires (block.timestamp > order.deadlineTimestamp)
​
// No one except the seller can call cancelSellOrder, so tokens remain locked forever
orderBook.cancelSellOrder(orderId); // Reverts for anyone except seller

Explanation: This PoC demonstrates how an expired order becomes unremovable if the seller is inactive, which could lead to permanent token lockup and contract clutter.

Recommended Mitigation

- if (order.seller != msg.sender) revert NotOrderSeller();
+ if (order.seller != msg.sender && block.timestamp < order.deadlineTimestamp) revert NotOrderSeller();
// Allow anyone to cancel an order if it is expired

Explanation: The mitigation allows anyone to cancel an order after its deadline has passed, ensuring that expired orders can always be cleaned up and locked tokens can be released, even if the seller is inactive.

Updates

Lead Judging Commences

yeahchibyke Lead Judge
7 days ago
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.