OrderBook

Description (important) :

• Note: Due to a technical limitation in the submission interface, we were unable to select the exact file path under the Scope section. As a workaround, we selected the root path (.../). However, please note that the actual affected file is: src/OrderBook.sol

Root Cause :

• The buyOrder() function in OrderBook.sol is vulnerable to a re-entrancy attack. This happens for two main reasons:

  1. The contract updates the totalFees state variable after making external calls, breaking the Checks-Effects-Interactions pattern.

  2. The contract allows any ERC20 token to be listed via setAllowedSellToken without verifying whether the token behaves safely. This opens the door for malicious tokens that can trigger re-entrant behavior in their transfer or transferFrom functions.

  Together, these design flaws can be exploited to drain funds from the contract using a specially crafted token.

Impact :

• This vulnerability allows a malicious actor to drain funds from the OrderBook contract. By adding a specially crafted ERC20 token with re-entrant behavior to the list of allowed tokens, the attacker can exploit the buyOrder() function multiple times within a single transaction. This can result in the attacker receiving more tokens than intended or causing locked tokens to be released repeatedly. If left unpatched, this flaw could lead to significant financial loss for the protocol or its users.

Normal Behavior :

• In normal operation, the buyOrder() function lets a buyer purchase tokens listed by a seller in exchange for a fixed amount of USDC. Once the payment is received, the contract transfers the seller’s tokens to the buyer, marks the order as inactive, and updates the protocol’s collected fees.

Specific Issue or Problem :

• The buyOrder() function makes multiple external token transfers before finishing all internal updates—most importantly, before updating totalFees. If the token being sold is malicious and has a re-entrant transfer function, it can call back into buyOrder() while the first call is still running. Even though the second call might revert due to isActive = false, the fact that re-entrancy is possible opens the door to logic manipulation and potential fund loss.

Risk :

Likelihood:

• This attack becomes possible if the contract owner mistakenly allows a malicious ERC20 token through setAllowedSellToken. If that token includes re-entrant logic in its transfer or transferFrom functions, the vulnerability can be triggered when someone tries to buy a sell order using buyOrder().

  Since the contract doesn't have general protections against re-entrancy (like a reentrancy guard), a malicious token can exploit this flow and interfere with internal state changes.

Impact:

• If a malicious token is allowed, an attacker can exploit the buyOrder() function to drain locked seller tokens or the contract’s USDC funds through repeated re-entrant calls. This could result in serious fund loss—either from the protocol’s collected fees or from users’ tokens.

  In some edge cases, the same sell order might even get filled more than once, leading to unexpected asset imbalance and broken assumptions in the protocol.

Proof of Concept :

Instructions to Run PoC:

1 - Create necessary directories:

In your project root, create the src/test-tokens directories.

2 - Place token files:

Place EvilToken.sol in src/test-tokens/EvilToken.sol.

Place MockERC20.sol in src/test-tokens/MockERC20.sol.

3 - Place the test file:

Place EvilToken.t.sol in test/EvilToken.t.sol.

4 - Install dependencies:

Ensure OpenZeppelin Contracts and forge-std are properly installed (run forge install).

5 - Run the test:

In your project terminal, execute the following command:

Expected Output :

The test test_ReentrancyAttackOnBuyOrder() is expected to PASS.
This means the Re-entrancy attack was successfully triggered, and the OrderBook contract failed to stop it.

You should see logs confirming that the EvilToken re-entered the buyOrder() function during execution.

Recommended Mitigation :

1. Add a Re-entrancy Guard
The most effective fix is to use a re-entrancy guard, like OpenZeppelin’s ReentrancyGuard.

How:
Make the OrderBook contract inherit from ReentrancyGuard and add the nonReentrant modifier to all sensitive functions, especially:

• buyOrder()

• cancelSellOrder()

• withdrawFees()

  
  These functions involve external token calls and are all vulnerable to re-entrancy.

**2. Follow the Checks-Effects-Interactions Pattern Carefully **
Update all internal state (like totalFees or order.isActive) before making any external token transfers. This protects against attackers who try to re-enter before the contract finishes updating its state.

3. Validate Tokens More Strictly
The setAllowedSellToken() function should only allow well-behaved tokens.
To prevent abuse:

• Reject tokens that have transfer fees, rebasing, or custom logic in transfer() or transferFrom().

• Consider adding a whitelist of known safe tokens or use an external validator to check token behavior before allowing them.