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

Lack of a Cancellation Mechanism Exposes Buyers to Forced Trades

pexy

Lack of a Cancellation Mechanism Exposes Buyers to Forced Trades

Description

  • The protocol does not provide a native function for users to cancel a buyOrder transaction that has been submitted to the mempool. Once a user broadcasts their intent to buy, it is irrevocable from their perspective.

  • This forces users who have made a mistake or changed their mind to rely on indirect methods like revoking token approvals, which can be easily circumvented by a malicious or rational miner. This architectural omission represents a critical lack of user agency and can lead to significant, forced financial losses.

@> 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();
...
}
​

Risk

Likelihood:

A user may wish to cancel a pending buyOrder for several valid reasons:

  • They entered the wrong orderId by mistake.

  • Market conditions have changed, making the trade no longer desirable.

  • They have detected that the target order is malicious (e.g., due to an ID preemption attack).

Impact:

  • In the current design, the user has no reliable way to prevent the transaction from executing. A rational miner can see both the user's buyOrder and their subsequent attempt to cancel (e.g., approve(..., 0)) in the mempool and can choose to ignore the cancellation, process the buyOrder, and claim the transaction fee. The protocol's design fails to provide users with a fundamental safety feature: the ability to retract an unconfirmed action.

Proof of Concept

The following scenario shows a user, Dan, who mistakenly submits a buyOrder for Bob's expensive, incorrect order. Dan has no way to cancel this mistaken transaction and is forced into a bad trade.

function test_audit_noCancellation() public {
// Alice creates a legitimate order that Dan wants to buy.
vm.startPrank(alice);
wbtc.approve(address(book), 1e8);
book.createSellOrder(address(wbtc), 1e8, 100_000e6, 2 days); // Order ID 1
vm.stopPrank();
​
// Bob creates an unrelated, expensive order for a different token.
vm.startPrank(bob);
weth.approve(address(book), 1e18);
uint256 bobOrderId = book.createSellOrder(address(weth), 1e18, 10_000e6, 2 days); // Order ID 2
vm.stopPrank();
​
// Dan mistakenly submits a transaction for Bob's order (ID 2) instead of Alice's (ID 1).
// Once this is in the mempool, he cannot retract it. A miner will execute it.
vm.startPrank(dan);
usdc.approve(address(book), 10_000e6);
book.buyOrder(bobOrderId);
vm.stopPrank();
​
// Assert the outcome: Dan was forced to buy the wrong asset at a bad price.
assertEq(weth.balanceOf(dan), 1e18, "Dan was forced to buy WETH");
assertEq(wbtc.balanceOf(dan), 0, "Dan did not get the WBTC he wanted");
}

Recommended Mitigation

To fully solve this, the protocol's architecture for trade execution must be redesigned. The recommended approach is to implement a commit-reveal scheme, which provides a natural and secure way for users to cancel their intent.

This involves replacing the direct buyOrder call with a two-step process:

  1. commit: The user submits a hash of their intent, hiding the details.

  2. reveal: The user reveals the details, and the trade executes.

A cancellation function can then be added to allow a user to securely delete their commitment before it is revealed.

+ // --- New State Variable ---
+ // Stores the user's hidden intention.
+ mapping(address => bytes32) public orderCommitments;
​
+ // --- New Events ---
+ event OrderCommitted(address indexed buyer, bytes32 indexed commitment);
+ event CommitmentCancelled(address indexed user, bytes32 indexed commitment);
​
+ function commitToBuyOrder(bytes32 _commitment) public {
+ require(_commitment != bytes32(0), "Commitment cannot be empty");
+ orderCommitments[msg.sender] = _commitment;
+ emit OrderCommitted(msg.sender, _commitment);
+ }
​
+ function cancelBuyCommitment() public {
+ // Verify that a commitment actually exists to be cancelled.
+ require(orderCommitments[msg.sender] != bytes32(0), "No commitment to cancel");
+ // By setting the commitment to zero, the check in `revealAndBuyOrder`
+ // will always fail. The intent is now irrevocably cancelled.
+ orderCommitments[msg.sender] = bytes32(0);
+ emit CommitmentCancelled(msg.sender, _commitment);
+ }
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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