OrderBook

First Flight #43
Beginner FriendlySolidity
100 EXP
Submission Details
Impact: low
Likelihood: high
Invalid

String-based revert used instead of custom error in emergencyWithdrawERC20

Author Revealed upon completion

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 contract uses custom errors for gas efficiency and clear revert reasons.
Description (issue):
In emergencyWithdrawERC20, it uses a string revert:
if (_tokenAddress == address(iWETH) || ...)
revert("Cannot withdraw core order book tokens via emergency function");
This is inconsistent and slightly more gas costly.
Risk
Likelihood:
Low.
Reason 1: Occurs when the owner tries to withdraw core tokens.
Reason 2: Design inconsistency only — no direct user exploit.
Impact:
Gas inefficiency.
Reduced audit clarity and code consistency.

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Impact 1

  • Impact 2

Proof of Concept

Explanation:
In emergencyWithdrawERC20, it uses a string revert:
if (_tokenAddress == address(iWETH) || ...)
revert("Cannot withdraw core order book tokens via emergency function");

Recommended Mitigation

- remove this code
+ add this code
+ error CannotWithdrawCoreToken();
- revert("Cannot withdraw core order book tokens via emergency function");
+ revert CannotWithdrawCoreToken();
Explanation:
This mitigation is about explicitly preventing reentrancy attacks on the buyOrder function.
---
⛓️ What is reentrancy?
Reentrancy is when an external contract is able to call back into your contract before the first function call finishes.
It can allow attackers to exploit inconsistent or unfinished internal states, drain funds, or bypass checks.
Famous example
The DAO hack on Ethereum in 2016 was a reentrancy attack, which allowed an attacker to withdraw funds repeatedly before balances were updated.
---
⚔️ Why is buyOrder a potential target?
In buyOrder, you do:
1️⃣ Mark the order as inactive.
2️⃣ Transfer USDC from buyer to seller.
3️⃣ Transfer the sold token to the buyer.
While you correctly mark order.isActive = false first (which protects against some simple reentrancy), if any future change adds a vulnerable external call before that, or if tokens with malicious callbacks are used, it could become vulnerable.
---
🛡️ What does nonReentrant do?
When you add nonReentrant, you use OpenZeppelin’s ReentrancyGuard, which works like this:
1️⃣ When a nonReentrant function is called, a status flag inside the contract is set to "entered."
2️⃣ If a reentrant call happens (a nested call into the same function or another nonReentrant function), the contract immediately reverts.
3️⃣ Once the outer function finishes, the flag resets to "not entered."
---
🔧 How does this protect you?
It forces your function to complete before it can be called again from within itself or another vulnerable path.
---
🟢 The actual change
- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId) public nonReentrant {
You simply add nonReentrant as a modifier to this function.
No further code changes needed inside the function itself.
---
✍️ Why is this mitigation important (even if you already set flags)?
✅ Future-proofing: Prevents future changes from introducing reentrancy bugs accidentally.
✅ Cleaner audits: Makes it very obvious to auditors and other devs that reentrancy is handled explicitly.
✅ Defense in depth: Even if your logic is safe today, new features (like supporting callbacks or hooks) might break assumptions.
---
⚡ Summary
Without mitigation With mitigation
Only relies on manual state checks Enforced at the EVM level via OpenZeppelin guard
Reentrancy bugs possible if code changes later Strong reentrancy lock automatically prevents nested calls
Higher audit burden Clear and safe design, easier to audit
---
✅ In one sentence
👉 Adding nonReentrant ensures buyOrder cannot be re-entered during execution, eliminating an entire class of potential attacks automatically.
---
💬 Let me know if you'd also like a visual diagram showing how a reentrancy exploit would work with and without this guard!
Updates

Lead Judging Commences

yeahchibyke Lead Judge
6 days ago
yeahchibyke Lead Judge about 5 hours ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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