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

Reentrancy Attack While Token Transfer

abhishek2416

Root + Impact

Multiple functions in the contract (createSellOrder, amendSellOrder, cancelSellOrder, buyOrder, emergencyWithdrawERC20, and withdrawFees) rely on external calls to ERC20 tokens via safeTransfer and safeTransferFrom from OpenZeppelin. These external token calls are made before internal state updates, which makes the contract vulnerable to reentrancy attacks — especially if the token implements malicious hooks, like fee-on-transfer mechanics or callbacks that invoke fallback logic during the transfer.

Impact:
A malicious token can trigger reentrant calls into the contract before the function completes, potentially calling createSellOrder() or similar again and manipulating shared state (like _nextOrderId, orders, totalFees, etc.).

  • This could result in:

    • Order duplication or overwriting

    • Incorrect fee accounting

    • Broken invariants or locked funds

    • Partial denial of service (clogging the system with invalid or unfulfillable orders)

Description

  • The contract allows users to create, amend, cancel, and buy orders using ERC20 tokens by calling safeTransfer or safeTransferFrom, assuming these functions only perform a token balance transfer.

  • However, several ERC20 tokens (especially on mainnet) may implement callback hooks, fee-on-transfer, or fallback functions that invoke external calls or execute arbitrary code during transfer or transferFrom. These external calls may recursively call back into the contract, especially before the function finishes and the state is updated.

// Root cause in the codebase with @> marks to highlight the relevant section
​
function createSellOrder(...) public returns (...) {
...
@> IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell); // Reentrancy entry point
...
}
​
function amendSellOrder(...) public {
...
@> token.safeTransferFrom(msg.sender, address(this), diff); // External call vulnerable
@> token.safeTransfer(order.seller, diff); // External call vulnerable
...
}
​
function cancelSellOrder(...) public {
...
@> IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell); // External call
...
}
​
function buyOrder(...) public {
...
@> iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee); // External call
@> iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives); // External call
@> IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell); // External call
...
}
​
function emergencyWithdrawERC20(...) external {
...
@> token.safeTransfer(_to, _amount); // External call
...
}
​
function withdrawFees(...) external {
...
@> iUSDC.safeTransfer(_to, totalFees); // External call
...
}
​

Risk

Likelihood:

  • When a malicious or specially-crafted ERC20 token is allowed through setAllowedSellToken() or passed to emergencyWithdrawERC20().

  • When a fee-on-transfer or reentrant token (like tokens with onTransfer() hooks or callbacks) is used in a legitimate order creation or amendment flow.

Impact:

  • An attacker can re-enter the contract through safeTransfer calls before internal state (like orderId, orders, totalFees) is updated, leading to:

    • Duplicate order creation (createSellOrder)

    • Misreported fees or balances

    • Re-entrance into amendSellOrder() or cancelSellOrder() leading to griefing attacks

  • Could lock user funds or cause order book corruption, making the protocol unsafe for mainnet usage.

Proof of Concept

This malicious token contract simulates a reentrancy attack by calling back into createSellOrder() during transferFrom(). Since the original createSellOrder() does not update state until after the token transfer, the second reentrant call can manipulate internal state like _nextOrderId, orders, or contract balance assumptions, causing state corruption or inconsistent behavior. Similar logic can be used to exploit amendSellOrder, cancelSellOrder, and others.

contract MaliciousToken is IERC20 {
address public orderBook;
bool public hasCalled;
​
constructor(address _orderBook) {
orderBook = _orderBook;
}
​
function transferFrom(address, address, uint256) external override returns (bool) {
if (!hasCalled) {
hasCalled = true;
// Reentrant call into orderBook to create another order
OrderBook(orderBook).createSellOrder(address(this), 1 ether, 1000, 1 days);
}
return true;
}
​
// Other IERC20 methods omitted for brevity
}
​

Recommended Mitigation

To prevent this type of attack, all functions that perform external calls to tokens using safeTransfer or safeTransferFrom must use the nonReentrant modifier (from OpenZeppelin's ReentrancyGuard). This modifier ensures that no nested (reentrant) call to the same function can occur during execution. This is the most reliable and widely adopted mitigation in DeFi to defend against ERC20 transfer-hook reentrancy.

+ function createSellOrder(...) public nonReentrant returns (...) {
+ function amendSellOrder(...) public nonReentrant {
+ function cancelSellOrder(...) public nonReentrant {
+ function buyOrder(...) public nonReentrant {
+ function emergencyWithdrawERC20(...) external nonReentrant {
+ function withdrawFees(...) external nonReentrant {
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 2 months ago
yeahchibyke Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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