OrderBook

First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Expired Orders Can Be Filled

a. Title

Expired orders can be purchased due to an incorrect logical condition in the deadline check.

b. Summary

The buyOrder function is intended to prevent the purchase of orders that have passed their deadline. However, the validation logic uses a strict greater-than comparison (>) instead of a greater-than-or-equal-to comparison (>=) for the timestamp check. This allows an order to be filled at the exact block timestamp of its expiration, a moment when it should be considered expired and invalid.

c. Description
In the OrderBook contract, sellers specify a deadlineTimestamp when creating or amending an order. This timestamp marks the point in time after which the order should no longer be available for purchase. The buyOrder function contains a check to enforce this, if (block.timestamp > order.deadlineTimestamp) revert OrderExpired();.

The issue arises because this check will pass if block.timestamp is exactly equal to order.deadlineTimestamp. In blockchain applications, time-based conditions must be handled precisely. An order should be considered expired if the current block timestamp is greater than or equal to its deadline. By allowing a transaction to proceed at the exact expiration timestamp, the contract does not behave as users would reasonably expect. A buyer could purchase an asset at a stale price, especially in volatile market conditions, believing the order was still valid when it should have been treated as expired.

d. Step-by-step Analysis

  1. A seller creates an order with a specific deadlineTimestamp.

  2. The market moves, and the price of the asset changes.

  3. A buyer observes that an order is at its exact expiration timestamp (block.timestamp == order.deadlineTimestamp).

  4. The buyer calls the buyOrder function.

  5. The check block.timestamp > order.deadlineTimestamp evaluates to false, so the transaction is not reverted.

  6. The buyer successfully purchases the tokens from the expired order, potentially at a price that is no longer favourable to the seller, although in this case, it is the buyer who is at risk if they are buying an asset whose price has fallen. The core issue is the violation of the time-lock guarantee.

e. Severity Classification

  • Impact: Medium. A buyer could unintentionally purchase an asset from an order that should have been expired. This could lead to financial loss if the asset's market price has dropped below the order price at the moment of expiration. It violates a core functional guarantee of the contract.

  • Likelihood: Low. An attacker or user would need to submit a transaction in the exact same block that the order expires. While this is possible, especially for sophisticated actors monitoring the mempool, it is not a trivial condition to meet.

f. File Name
src/OrderBook.sol

g. Code Quote

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();
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);

h. Recommendation
The conditional statement should be modified to include an equality check, ensuring that orders cannot be filled at or after their expiration timestamp. The check should be changed from block.timestamp > order.deadlineTimestamp to block.timestamp >= order.deadlineTimestamp.

Recommended Code:

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();
order.isActive = false;
// ...
}

This change ensures that once the block timestamp reaches the deadlineTimestamp, the order is correctly identified as expired and can no longer be filled, aligning the contract's behaviour with user expectations.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.