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

Stale/Locked Tokens from Expired Orders

nagatejakachapuram

Description


The OrderBook contract does not automatically return tokens from expired orders. Sellers must manually call cancelSellOrder to retrieve their tokens after an order's deadlineTimestamp has passed. This can lead to tokens becoming permanently locked within the contract if sellers fail to perform this manual cancellation.


Risk


Root Cause: The contract relies solely on explicit seller action (cancelSellOrder) to retrieve tokens from orders that have passed their deadline, leading to potential token stagnation.

Solidity

// In buyOrder:
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// ...
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Prevents buying
// ...
}
​
// @> In cancelSellOrder (as currently implemented, requires seller action):
function cancelSellOrder(uint256 _orderId) public {
// ... (Seller must call this to retrieve tokens)
order.isActive = false;
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
// ...
}
​

Likelihood: Medium. Users frequently forget or deem it uneconomical (due to gas fees) to perform cleanup transactions for small or expired amounts.

Impact: Low to Medium.

  • Economic Inefficiency: Seller's capital remains idle and inaccessible within the contract.

  • Poor User Experience: Sellers face additional steps and potential gas costs to retrieve their own funds.

  • Misleading TVL: The contract might show a higher Total Value Locked (TVL) than what is actively tradable, due to locked, expired tokens.


Proof of Concept


  1. Seller A creates an order for 100 WETH with a deadlineTimestamp.

  2. The deadlineTimestamp passes, and the order expires.

  3. No one can fill Seller A's order.

  4. Seller A does not call cancelSellOrder(orderId).

  5. Result: The 100 WETH remains locked indefinitely in the OrderBook contract, inaccessible to Seller A without a manual cancellation.


Recommended Mitigation


Introduce a dedicated function or modify cancelSellOrder to explicitly facilitate claiming tokens from expired orders without requiring an "active" status beyond the deadline.

Option 1 (Modify cancelSellOrder): Adjust the logic to allow cancellation for both active orders and those that are expired but not yet marked inactive.

Diff

function cancelSellOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
​
if (order.seller == address(0)) revert OrderNotFound();
if (order.seller != msg.sender) revert NotOrderSeller();
- if (!order.isActive) revert OrderAlreadyInactive(); // This prevents cancelling (claiming) expired but un-cancelled orders
+ // Allow cancellation if order is active OR if it's expired
+ if (!order.isActive && block.timestamp < order.deadlineTimestamp) revert OrderAlreadyInactive(); // Only revert if inactive AND NOT expired yet
​
order.isActive = false; // Mark as inactive (if not already)
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
​
emit OrderCancelled(_orderId, order.seller);
}
​

Option 2 (New claimExpiredOrder function - cleaner separation): Add a specific function for claiming only expired orders.

Solidity

// Add this new function to the contract
function claimExpiredOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
​
if (order.seller == address(0)) revert OrderNotFound();
if (order.seller != msg.sender) revert NotOrderSeller();
if (!order.isActive) revert OrderNotActive(); // Must be active to be claimed (i.e., not already filled/cancelled)
if (block.timestamp < order.deadlineTimestamp) revert InvalidDeadline(); // Must be past deadline
​
order.isActive = false; // Mark as inactive/claimed
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
​
emit OrderCancelled(_orderId, order.seller); // Can reuse or create a new event like OrderClaimed
}
​

Reference Files:

  • src/OrderBook.sol

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month 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.