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

[L-01] Lack of function to cleanup expired orders causes tokens deposited for orders that have are now expired to be locked forever unless cancelled manually by seller

soarinskysagar

Root + Impact

Description

  • All sell orders have an expiration timestamp. Once this expiration timestamp is passed, amendment and purchase of these orders ar enot permitted.

  • However, there are no functions to deactivate order and return the locked tokens to the seller for the concerned order. Only the user can manually call cancelOrder() to deactivate the order and claim locked tokens

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
if (!allowedSellToken[_tokenToSell]) revert InvalidToken();
if (_amountToSell == 0) revert InvalidAmount();
if (_priceInUSDC == 0) revert InvalidPrice();
@> if (_deadlineDuration == 0 || _deadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
​
@> uint256 deadlineTimestamp = block.timestamp + _deadlineDuration;

Risk

Likelihood:

  • Whenever an order expires without getting bought

Impact:

  • Tokens are locked and have to be manually claimed through cancelOrder(), which can be done only by user

Proof of Concept

Place the following function into test/TestOrderBook.t.sol and run with forge test --mt test_expiredOrders

function test_expiredOrders() public {
// alice creates sell order for wbtc
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 2 days);
vm.stopPrank();
​
assert(aliceId == 1);
assert(wbtc.balanceOf(alice) == 0);
assert(wbtc.balanceOf(address(book)) == 2e8);
​
// warp to 3 days later, order should have expired due to duration being 2 days
vm.warp(block.timestamp + 3 days);
​
vm.startPrank(dan);
usdc.approve(address(book), 200_000e6);
// expect tx to fail due to expired duration
vm.expectRevert();
book.buyOrder(aliceId);
vm.stopPrank();
​
// cancel order to claim tokens
vm.startPrank(alice);
book.cancelSellOrder(aliceId);
}

The test passes, showing that the issue is indeed real.

Recommended Mitigation

Add a sweepExpiredOrder() function to clean up expired orders and return the tokens to users. This function may be triggered by a centralised entity

+function sweepExpiredOrders() public {
+ for (uint256 _orderId = 1; _orderId < _nextOrderId; _orderId++) {
+ Order storage order = orders[_orderId];
+
+ if (block.timestamp >= order.deadlineTimestamp) {
+ order.isActive = false;
+ IERC20 token = IERC20(order.tokenToSell);
+ token.safeTransfer(order.seller, order.amountToSell);
+ }
+ }
+}
Updates

Lead Judging Commences

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