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

Expired Orders Can Become Unrecoverable if Seller Loses Access

pexy

Expired Orders Can Become Unrecoverable if Seller Loses Access

Description

  • The cancelSellOrder function allows the original seller to cancel an active order and reclaim their locked collateral.

  • However, this function is restricted exclusively to the order.seller. If an order expires and the seller's account becomes permanently inaccessible (e.g., due to lost private keys or a bug in a contract-based seller), there is no alternative mechanism for anyone to cancel the order. This results in the underlying collateral being permanently locked within the contract.

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();
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:

  • This occurs if the seller's account becomes permanently inaccessible due to events like private key loss or a bug in a contract-based seller.

Impact:

  • The seller's collateral for the expired order will be permanently and irrecoverably lost.

  • The protocol accumulates unresolvable "zombie" orders on-chain, which can lead to state bloat.

Proof of Concept

// Scenario 1: A user creates a sell order for a large amount of WBTC and then loses access to their private key.
// After the order expires, the WBTC collateral cannot be recovered by anyone.
​
// Scenario 2: An attacker creates thousands of dust-amount sell orders and then abandons the address.
// These expired orders remain in the contract's state forever, as they can never be cancelled, leading to state bloat.
​

Recommended Mitigation

Add a separate, public-facing function that can only be called after a significant grace period has passed since the order's expiry. This ensures funds can be recovered without introducing new risks.

+ uint256 public constant EXPIRY_GRACE_PERIOD = 30 days;
​
+ function reclaimExpiredOrder(uint256 _orderId) public {
+ Order storage order = orders[_orderId];
+
+ if (order.seller == address(0)) revert OrderNotFound();
+ if (!order.isActive) revert OrderAlreadyInactive();
+
+ // Check that the order is not just expired, but long-expired.
+ require(block.timestamp > order.deadlineTimestamp + EXPIRY_GRACE_PERIOD, "Grace period not over");
+
+ // Mark as inactive
+ order.isActive = false;
+
+ // Return locked tokens to the original seller
+ IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
+
+ emit OrderCancelled(_orderId, order.seller);
+ }
Updates

Lead Judging Commences

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

Give us feedback!