Submission Details
Impact:
Likelihood:
Missing nonReentrant modifier on buyOrder, Lack of explicit reentrancy protection on buyOrder
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 (normal behavior):
The buyOrder function allows a buyer to purchase tokens from an active order, transferring USDC and the selling token atomically. It updates the order status before transferring funds.
Description (issue):
The function currently does not use a nonReentrant modifier. Although the order status is set to inactive before external calls, future modifications or external contract integrations could reintroduce reentrancy risk.
function buyOrder(uint256 _orderId) public {
...
order.isActive = false; // @> State updated here
...
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
}
Risk
Likelihood:
Medium.
This becomes exploitable if future logic introduces callbacks, external hooks, or if an ERC20 token’s transfer triggers external code execution.
Impact:
Potential loss of tokens or double-spending of orders if reentrancy is exploited.
Could lead to draining of tokens from the contract or unintended fills.
Risk
Likelihood:
-
Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
-
Reason 2
Impact:
-
Impact 1
-
Impact 2
Proof of Concept
Explanation: While currently safe, adding an example hypothetical scenario:
// Malicious token contract
function transfer(address recipient, uint256 amount) external returns (bool) {
// During a transfer from OrderBook, attacker re-enters buyOrder()
}
Recommended Mitigation
- remove this code
+ add this code
+ import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
- contract OrderBook is Ownable {
+ contract OrderBook is Ownable, ReentrancyGuard {
- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId) public nonReentrant {
Explanation:
---
✅ The problem we are addressing
The original buyOrder function does not explicitly protect against reentrancy attacks.
What is a reentrancy attack?
A malicious external contract can call back into your contract before the first call finishes, potentially messing with your internal state or draining funds.
Famous example: The DAO hack on Ethereum (2016).
Why is it relevant here?
Your buyOrder function involves:
Updating order state
Transferring ERC20 tokens (which can call external code in non-standard tokens)
Handling user-controlled addresses
Currently, you do update order.isActive before transfers, which is good, but explicit prevention (defense-in-depth) is considered best practice in DeFi.
---
🔬 Mitigation explained line by line
1️⃣ Add ReentrancyGuard import
+import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
What is it?
A standard OpenZeppelin module that helps prevent reentrancy by using a status flag internally.
It defines a modifier called nonReentrant that can be applied to functions.
---
2️⃣ Modify contract inheritance
- contract OrderBook is Ownable {
+ contract OrderBook is Ownable, ReentrancyGuard {
What does this do?
Your contract now inherits ReentrancyGuard.
This enables you to use the nonReentrant modifier on any function you choose.
---
3️⃣ Add nonReentrant modifier to buyOrder
- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId) public nonReentrant {
What does nonReentrant do?
When buyOrder starts execution, it sets an internal status flag to "entered."
If, during that call, something tries to call buyOrder (or any other function using nonReentrant) again before the first call finishes, it reverts immediately.
After the function finishes, it resets the status to "not entered."
---
✅ Why is this a good mitigation?
Even if a malicious token or contract tries to re-enter during an external call (e.g., during safeTransfer), the second call will fail.
This explicitly protects the contract and signals to future maintainers and auditors that reentrancy has been considered.
---
🟢 Summary of each step
Change Explanation
import {ReentrancyGuard} Pulls in the OpenZeppelin helper for reentrancy protection.
, ReentrancyGuard in inheritance Adds reentrancy lock functionality to your contract.
nonReentrant on buyOrder Explicitly prevents nested (reentrant) calls during execution.
---
💬 Final note
Even though your original logic is fairly safe due to early state updates, adding nonReentrant is considered a best practice in DeFi, especially for high-value protocols.
---
✅ Let me know if you'd also like to mark other functions (e.g., withdrawFees) as nonReentrant, or see a diagram of how reentrancy works step by step!
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.