OrderBook

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

Missing Order Expiration Automation Causes Indefinite Fund Locking

Missing Order Expiration Automation

Description

  • The OrderBook contract allows sellers to create orders with deadlines, but does not provide any mechanism to automatically cancel expired orders

  • Expired orders remain in an "active" state indefinitely, with seller funds locked in the contract until manual cancellation occurs

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();
// @> No automatic expiration - sellers must manually cancel expired orders
order.isActive = false;
IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
emit OrderCancelled(_orderId, order.seller);
}
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // @> Prevents buying but doesn't auto-cancel
// ... rest of function
}

Risk

Likelihood: Medium

  • Users may forget to cancel expired orders, especially for long-term positions

  • Sellers may abandon their accounts or lose access to private keys

  • No external incentives exist for third parties to help cancel expired orders

  • The problem compounds over time as more orders expire without cancellation

Impact: Medium

  • Seller funds remain locked indefinitely in the contract

  • Reduced liquidity for sellers who cannot access their tokens

  • Contract accumulates "zombie" orders that serve no purpose

  • Poor user experience for abandoned or forgotten orders

  • Potential for permanent fund loss if sellers lose access to their accounts

Proof of Concept

function test_missingExpirationAutomation() public {
console2.log("Initial balances from setUp():");
console2.log("Alice WBTC balance: %d", wbtc.balanceOf(alice));
console2.log("Bob WETH balance: %d", weth.balanceOf(bob));
console2.log("Clara WSOL balance: %d", wsol.balanceOf(clara));
vm.startPrank(alice);
wbtc.approve(address(book), 1e8);
uint256 shortTermOrder = book.createSellOrder(
address(wbtc),
1e8, // 1 WBTC
80000e6, // 80,000 USDC
1 hours
);
vm.stopPrank();
vm.startPrank(bob);
weth.approve(address(book), 1e18);
uint256 mediumTermOrder = book.createSellOrder(
address(weth),
1e18, // 1 WETH
2500e6, // 2,500 USDC
1 days
);
vm.stopPrank();
vm.startPrank(clara);
wsol.approve(address(book), 1e18);
uint256 longTermOrder = book.createSellOrder(
address(wsol),
1e18, // 1 WSOL
150e6, // 150 USDC
3 days
);
vm.stopPrank();
// Record balances after order creation
uint256 aliceWBTCAfter = wbtc.balanceOf(alice);
uint256 bobWETHAfter = weth.balanceOf(bob);
uint256 claraWSOLAfter = wsol.balanceOf(clara);
console2.log("\nBalances after order creation:");
console2.log(
"Alice WBTC balance: %d (locked: %d)",
aliceWBTCAfter,
1e8
);
console2.log("Bob WETH balance: %d (locked: %d)", bobWETHAfter, 1e18);
console2.log(
"Clara WSOL balance: %d (locked: %d)",
claraWSOLAfter,
1e18
);
// Verify tokens are locked in contract
assertEq(
wbtc.balanceOf(address(book)),
1e8,
"Contract should hold Alice's WBTC"
);
assertEq(
weth.balanceOf(address(book)),
1e18,
"Contract should hold Bob's WETH"
);
assertEq(
wsol.balanceOf(address(book)),
1e18,
"Contract should hold Clara's WSOL"
);
// Fast forward past all deadlines
vm.warp(block.timestamp + 4 days);
console2.log("\n=== AFTER ALL ORDERS EXPIRED ===");
console2.log("Current timestamp: %d", block.timestamp);
// Verify all orders are expired but still "active"
OrderBook.Order memory order1 = book.getOrder(shortTermOrder);
OrderBook.Order memory order2 = book.getOrder(mediumTermOrder);
OrderBook.Order memory order3 = book.getOrder(longTermOrder);
assertTrue(order1.isActive, "Order1 should still be marked active");
assertTrue(order2.isActive, "Order2 should still be marked active");
assertTrue(order3.isActive, "Order3 should still be marked active");
assertTrue(
block.timestamp >= order1.deadlineTimestamp,
"Order1 should be expired"
);
assertTrue(
block.timestamp >= order2.deadlineTimestamp,
"Order2 should be expired"
);
assertTrue(
block.timestamp >= order3.deadlineTimestamp,
"Order3 should be expired"
);
console2.log("All orders are expired but still marked as active");
// Demonstrate that buyers cannot purchase expired orders
vm.startPrank(dan);
usdc.approve(address(book), 200000e6);
vm.expectRevert(OrderBook.OrderExpired.selector);
book.buyOrder(shortTermOrder);
vm.expectRevert(OrderBook.OrderExpired.selector);
book.buyOrder(mediumTermOrder);
vm.expectRevert(OrderBook.OrderExpired.selector);
book.buyOrder(longTermOrder);
vm.stopPrank();
// But funds remain locked despite orders being expired and unbuyable
console2.log(
"Contract WBTC balance: %d",
wbtc.balanceOf(address(book))
);
console2.log(
"Contract WETH balance: %d",
weth.balanceOf(address(book))
);
console2.log(
"Contract WSOL balance: %d",
wsol.balanceOf(address(book))
);
assertEq(
wbtc.balanceOf(address(book)),
1e8,
"WBTC still locked in contract"
);
assertEq(
weth.balanceOf(address(book)),
1e18,
"WETH still locked in contract"
);
assertEq(
wsol.balanceOf(address(book)),
1e18,
"WSOL still locked in contract"
);
// Only Clara cancels her order
vm.prank(clara);
book.cancelSellOrder(longTermOrder);
console2.log("\nAfter Clara's manual cancellation:");
console2.log("Clara WSOL balance: %d", wsol.balanceOf(clara));
console2.log(
"Contract WSOL balance: %d",
wsol.balanceOf(address(book))
);
assertEq(
wsol.balanceOf(address(book)),
0,
"Contract should have no WSOL after cancellation"
);
// Alice and Bob funds remain permanently locked
console2.log(
"Alice WBTC balance: %d (lost: %d WBTC)",
wbtc.balanceOf(alice),
1e8
);
console2.log(
"Bob WETH balance: %d (lost: %d WETH)",
weth.balanceOf(bob),
1e18
);
console2.log(
"Contract WBTC balance: %d (permanently locked)",
wbtc.balanceOf(address(book))
);
console2.log(
"Contract WETH balance: %d (permanently locked)",
weth.balanceOf(address(book))
);
// These funds are permanently inaccessible
assertEq(wbtc.balanceOf(address(book)), 1e8, "WBTC permanently locked");
assertEq(
weth.balanceOf(address(book)),
1e18,
"WETH permanently locked"
);
// Show that orders appear expired but remain active
string memory order1Status = book.getOrderDetailsString(shortTermOrder);
string memory order2Status = book.getOrderDetailsString(
mediumTermOrder
);
console2.log("\n=== ORDER STATUS PARADOX ===");
console2.log(
"Order1 (Alice): isActive=%s, expired=%s",
order1.isActive ? "true" : "false",
block.timestamp >= order1.deadlineTimestamp ? "true" : "false"
);
console2.log(
"Order2 (Bob): isActive=%s, expired=%s",
order2.isActive ? "true" : "false",
block.timestamp >= order2.deadlineTimestamp ? "true" : "false"
);
// Calculate the economic impact
uint256 totalLockedValue = 80000e6 + 2500e6; // Alice's + Bob's order values
console2.log("\nEconomic impact:");
console2.log(
"Total value locked: %d USDC worth of tokens",
totalLockedValue / 1e6
);
console2.log("Success rate: 1/3 orders recovered (33%%)");
console2.log("Failure rate: 2/3 orders permanently locked (67%%)");
}

Recommended Mitigation

+ // Add batch cancellation for expired orders
+ function cancelExpiredOrders(uint256[] calldata _orderIds) external {
+ for (uint256 i = 0; i < _orderIds.length; i++) {
+ uint256 orderId = _orderIds[i];
+ Order storage order = orders[orderId];
+
+ // Skip if order doesn't exist or is already inactive
+ if (order.seller == address(0) || !order.isActive) {
+ continue;
+ }
+
+ // Only cancel if truly expired
+ if (block.timestamp >= order.deadlineTimestamp) {
+ order.isActive = false;
+ IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
+ emit OrderCancelled(orderId, order.seller);
+ }
+ }
+ }
+ // Add incentivized cancellation for community participation
+ function cancelExpiredOrderWithReward(uint256 _orderId) external {
+ Order storage order = orders[_orderId];
+
+ require(order.seller != address(0), "Order not found");
+ require(order.isActive, "Order already inactive");
+ require(block.timestamp >= order.deadlineTimestamp, "Order not expired");
+
+ order.isActive = false;
+
+ // Small reward for the caller (from protocol fees)
+ uint256 reward = 1e6; // 1 USDC reward
+ if (totalFees >= reward) {
+ iUSDC.safeTransfer(msg.sender, reward);
+ totalFees -= reward;
+ }
+
+ IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
+ emit OrderCancelled(_orderId, order.seller);
+ }
+ // Add view function to find expired orders
+ function getExpiredOrders(uint256 _startId, uint256 _endId) external view returns (uint256[] memory expiredIds) {
+ uint256 count = 0;
+
+ // First pass: count expired orders
+ for (uint256 i = _startId; i <= _endId && i < _nextOrderId; i++) {
+ Order storage order = orders[i];
+ if (order.seller != address(0) && order.isActive && block.timestamp >= order.deadlineTimestamp) {
+ count++;
+ }
+ }
+
+ // Second pass: populate array
+ expiredIds = new uint256[](count);
+ uint256 index = 0;
+ for (uint256 i = _startId; i <= _endId && i < _nextOrderId; i++) {
+ Order storage order = orders[i];
+ if (order.seller != address(0) && order.isActive && block.timestamp >= order.deadlineTimestamp) {
+ expiredIds[index] = i;
+ index++;
+ }
+ }
+ }
+ // Add events for better tracking
+ event ExpiredOrderCancelled(uint256 indexed orderId, address indexed canceller, address indexed seller);

Benefits of Mitigation:

  1. Prevents fund locking - Expired orders can be cancelled by anyone

  2. Improves user experience - Automated cleanup of expired orders

  3. Reduces contract bloat - Removes unusable "zombie" orders

  4. Incentivizes maintenance - Optional rewards for third-party cancellation

  5. Provides discovery tools - Helper functions to find expired orders

  6. Community-driven - Allows any user to help clean up expired orders

Alternative Solutions:

  1. Automatic state transitions - Orders automatically become inactive after expiration

  2. Grace period mechanism - Allow short window for manual cancellation before auto-expiration

  3. Keeper network integration - Use external automation services for order cleanup

  4. Batch processing - Periodic cleanup of expired orders through scheduled transactions

Updates

Lead Judging Commences

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