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

Reentrancy Potential Due to External Token Transfers in Critical Functions

Author Revealed upon completion

Root + Impact

DESCRIPTION

The OrderBook contract performs external ERC20 token transfers before updating internal state like order.isActive. This opens the door for reentrancy, where a malicious token contract could execute fallback logic and reenter sensitive functions leading to state manipulation, double withdrawals, or logic corruption.


Root Cause

  • In multiple functions, token transfers (via SafeERC20.safeTransfer or safeTransferFrom) are executed before the internal state is updated (e.g., isActive flags, fee accounting, order invalidation).

  • For example, in buyOrder(), tokens are transferred to msg.sender before finalizing state, allowing reentrancy vectors.


Impact

  • Reentrancy attacks could manipulate state mid-execution.

  • A malicious token (e.g., with fallback() calling cancelSellOrder) could interrupt and interfere with state updates.

  • If order is set inactive after token transfer, recursive token callbacks may exploit assumptions or cause double-send bugs.

@> IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell); // Reentrancy trigger point #1
@> iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee); // Reentrancy trigger point #2
@> IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell); // Reentrancy trigger point#3
@> iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives); // Reentrancy trigger point#4

Risk

Risk Likelihood

This vulnerability can occur during any token transfer where:

  • The contract interacts with external ERC20 contracts

  • The ERC20 includes malicious fallback, approve() hooks, or is malformed

Impact

  • Trade failures, potential double refunds, fee bypass

  • If attacker can execute functions like cancelSellOrder or recursively trigger buyOrder, impact multiplies

Proof of Concept

contract MaliciousToken is IERC20 {
address public orderBook;
​
constructor(address _orderBook) {
orderBook = _orderBook;
}
​
function transfer(address to, uint256 amount) external override returns (bool) {
// Reenter into OrderBook before transfer completes
OrderBook(orderBook).cancelSellOrder(1); // Can modify shared state
return true;
}}
​
// Other required ERC20 functions omitted for brevity
​

DESCRIPTION

The attacker whitelists MaliciousToken, creates an order, and waits for a buyer to call buyOrder().
During safeTransfer of tokens to buyer, the malicious token reenters the contract and calls cancelSellOrder()
or another mutating function. This executes before isActive is updated, leading to unpredictable state.


Recommended Mitigation


DESCRIPTION

This ensures that even if a token tries to reenter mid-transfer,
the contract state is already locked or updated, preventing recursive execution.
This approach aligns with DeFi best practices and mitigates both economic and logical reentrancy.

1) Use ReentrancyGuard
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract OrderBook is Ownable, ReentrancyGuard {
​
​
2)Apply nonReentrant modifier
function buyOrder(uint256 _orderId) public nonReentrant { ... }
​
function cancelSellOrder(uint256 _orderId) public nonReentrant { ... }
Updates

Lead Judging Commences

yeahchibyke Lead Judge
5 days ago
yeahchibyke Lead Judge 1 day ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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