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

State Update After External Calls Violates CEI Pattern in buyOrder Function

shieldrey

Root + Impact

Description

The buyOrder() function should follow the Checks-Effects-Interactions (CEI) pattern to prevent reentrancy attacks and ensure state consistency. However, the function updates the critical totalFees state variable after making external token transfers, violating this security pattern.

If a malicious seller contract implements a fallback function that triggers during the USDC transfer, it can reenter the OrderBook contract while totalFees is still in an inconsistent state. This creates a window where the protocol fees have been collected but not yet recorded in the contract's accounting system.

// Root cause in the codebase with @> marks to highlight the relevant section
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Checks and partial effects
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
​
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
​
// @> External interactions happen before all state updates
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives); // @> Reentrancy vector
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
​
// @> Critical state update happens AFTER external calls - CEI violation
totalFees += protocolFee;
​
emit OrderFilled(_orderId, msg.sender, order.seller);
}

Risk

Likelihood:

  • Occurs when a seller is a malicious contract that implements fallback/receive functions

  • Standard ERC-20 tokens like USDC don't typically have hooks, but custom tokens or upgradeable contracts could introduce them

  • The violation exists in every order execution, creating a consistent attack surface

Impact:

  • Fee accounting corruption where fees are collected but not recorded in totalFees

  • Potential for complex reentrancy attacks that manipulate contract state during the inconsistent window

  • Protocol revenue loss due to incorrect fee tracking

  • Broken invariants that could affect other contract functions relying on totalFees

Proof of Concept

contract MaliciousSellerContract {
OrderBook public orderBook;
uint256 public attackOrderId;
function createMaliciousOrder(address tokenToSell, uint256 amount, uint256 price) external {
// Approve tokens and create order
IERC20(tokenToSell).approve(address(orderBook), amount);
attackOrderId = orderBook.createSellOrder(tokenToSell, amount, price, 1 days);
}
// Fallback triggered during USDC transfer
fallback() external {
// At this point, totalFees hasn't been updated yet
// Malicious logic can execute while contract is in inconsistent state
// Could potentially call other functions or manipulate state
}
}

Recommended Mitigation

difffunction buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Checks
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
​
// Effects - ALL state changes first
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
+ totalFees += protocolFee;
​
// Interactions - external calls last
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
​
- totalFees += protocolFee;
​
emit OrderFilled(_orderId, msg.sender, order.seller);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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