Submission Details
Impact:
Likelihood:
Vulnerability: Funds locked (Denial of Service for seller's assets)
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
// Root cause in the codebase with @> marks to highlight the relevant section
Description:
The OrderBook contract is designed to facilitate peer-to-peer trading. Sellers create orders by locking their tokens in the contract, expecting buyers to fill them within a specified deadline. Normally, a seller should be able to cancel their order and retrieve their locked tokens if the order is no longer active (either filled or cancelled by the seller).
The specific issue is that the cancelSellOrder function prevents the seller from cancelling an order if its deadlineTimestamp has passed. This means if an order expires without being filled by a buyer, the seller's tokens remain permanently locked within the OrderBook contract, becoming irretrievable.
Risk:
Likelihood:
This will occur when a seller's order is created and subsequently expires because it is not filled by a buyer before its deadlineTimestamp. This is a common and expected outcome for many orders on an order book that may not find a match.
This will also occur if a seller attempts to cancel an order after its deadline has passed, regardless of whether a buyer was found.
Impact:
Loss of User Funds: The seller's amountToSell tokens are permanently trapped within the OrderBook contract. The seller loses access to these assets, and they cannot be retrieved by any party.
Denial of Service (for specific funds): The tokens associated with an expired order are rendered unusable and cannot participate in any further trades or be withdrawn by their rightful owner.
This ties in with two or three of my other reports this is the maximum effect I have found so far.
Risk
Likelihood:
-
Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
-
Reason 2
Impact:
-
Impact 1
-
Impact 2
Proof of Concept
// Root cause in the codebase with @> marks to highlight the relevant section
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 (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // This line prevents cancellation of expired orders
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);
}
Explanation:
Proof of Concept:
Deploy the OrderBook.sol contract and a MockERC20.sol token (e.g., MockWETH) on a local Anvil instance.
Fund Account A (seller) with some MockWETH.
Account A calls MockWETH.approve(<OrderBook_Address>, <amount>) to allow the OrderBook to transfer their MockWETH.
Account A calls OrderBook.createSellOrder(MockWETH_Address, 1e18, 5000e6, 60) (selling 1 WETH for 5000 USDC, with a deadline of 60 seconds). Note the returned orderId (e.g., 1).
Advance the blockchain time past the 60-second deadline using anvil_increaseTime (e.g., cast rpc anvil_increaseTime 120).
Account A attempts to call OrderBook.cancelSellOrder(1).
The transaction will revert with the error OrderExpired(), demonstrating that the seller cannot cancel their now-expired order.
The 1 MockWETH remains locked in the OrderBook contract, inaccessible to Account A.
Recommended Mitigation
- remove this code
+ add this code
Explanation:
Recommended Mitigation
The cancelSellOrder function should allow a seller to retrieve their tokens even if the order has expired. The check for order.isActive is sufficient to determine if the order has already been filled or explicitly cancelled
--- a/OrderBook.sol
+++ b/OrderBook.sol
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (order.seller != msg.sender) revert NotOrderSeller();
- if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Cannot amend expired order
+ // Consider removing or modifying: if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Or allow cancellation of expired orders
if (_newAmountToSell == 0) revert InvalidAmount();
if (_newPriceInUSDC == 0) revert InvalidPrice();
if (_newDeadlineDuration == 0 || _newDeadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
// 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)
+ if (!order.isActive) revert OrderAlreadyInactive(); // Ensures it's not already filled or cancelled
// Mark as inactive
order.isActive = false;
Updates
Appeal created
deadmanxxxii
9 days ago
deadmanxxxii
9 days ago
yeahchibyke
8 days ago
yeahchibyke
8 days ago
Rewards breakdown
nSLOC
217This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.