Root + Impact
Description
-
A user could frontrun an incoming buyOrder() transaction by calling cancelSellOrder(), causing the buyer’s transaction to revert or fail unexpectedly.
-
The contract lacks protection against front-running of order cancellation.
-
If order.isActive becomes false between mempool submission and execution, it can lead to failed trades.
if (!order.isActive) revert OrderNotActive();
Risk
>When a buyer sends a buyOrder(orderId) transaction, it enters the public mempool.
>A malicious actor—often the seller—can detect this pending transaction and immediately send a cancelSellOrder(orderId) before the buyer’s tx gets mined.
>Since both functions are publicly accessible and there is no mutex, nonce lock, or mempool shielding, this race condition is consistently exploitable, especially for high-value trades.
Proof of Concept
orderBook.buyOrder(orderId);
orderBook.cancelSellOrder(orderId);
OrderNotActive();
Explanation:
PoC Explanation: This vulnerability occurs because all Ethereum transactions are visible in the public mempool before they're mined.
When a buyer submits buyOrder(orderId), any malicious actor—including the seller—can observe it and immediately submit a cancelSellOrder(orderId) targeting the same orderId.
Since cancelSellOrder() is allowed for any active order owned by the caller, it will execute before the buy transaction if timed right, flipping isActive to false.
This causes the buyer’s tx to revert due to the require(order.isActive) check at the start of buyOrder().
As a result, the buyer suffers gas loss and failed execution, compromising UX and reliability.
Recommended Mitigation
1)
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract OrderBook is Ownable, ReentrancyGuard {
...
}
2)Apply the nonReentrant modifier to buyOrder() and cancelSellOrder()
function buyOrder(uint256 _orderId) public nonReentrant {
...
}
function cancelSellOrder(uint256 _orderId) public nonReentrant {
...
}
Mitigation Explanation:
To prevent front-running race conditions, the contract should enforce atomicity and internal state consistency using
the nonReentrant modifier.
This protects against nested execution that can arise during the critical window between order.isActive checks and external calls
. Additionally, updating order.isActive = false before initiating any safeTransferFrom or safeTransfer ensures consistent behavior even in concurrent transaction environments.
These changes fortify the contract against mempool-based timing attacks and ensure users experience predictable transaction behavior.
