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

CEI Pattern Violation in buyOrder() Enables Reentrancy and Fee Manipulation

lefeveje

CEI Pattern Violation in buyOrder() Enables Reentrancy and Fee Manipulation

Description

The buyOrder() function should safely execute a token purchase by collecting USDC payment (including protocol fees), transferring tokens to the buyer, paying the seller, and updating fee accounting atomically without risk of reentrancy.

Specific Issue

The function violates the Checks-Effects-Interactions (CEI) pattern by making multiple external calls before completing the final state update (totalFees += protocolFee), creating a reentrancy vulnerability that allows malicious contracts to manipulate fee accounting and potentially exploit the order execution process.

function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// ... validation checks ...
​
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
​
// @> MULTIPLE EXTERNAL CALLS BEFORE FINAL STATE UPDATE
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
​
// @> CRITICAL STATE UPDATE AFTER EXTERNAL CALLS
totalFees += protocolFee;
​
emit OrderFilled(_orderId, msg.sender, order.seller);
}

Risk

Likelihood:

  • USDC and established tokens (wETH, wBTC, wSOL) have standard implementations

  • These legitimate tokens are unlikely to contain malicious reentrancy code

  • However, the vulnerability exists and could be exploited if any involved token had non-standard behavior

  • Most critical function for value exchange makes this a high-priority fix

Impact:

  • Malicious USDC or token contracts could re-enter during any of the three transfers

  • totalFees accounting manipulation - attacker could prevent proper fee tracking

  • Could potentially execute multiple orders or manipulate protocol revenue

  • Most severe impact among all CEI violations due to direct value exchange

Proof of Concept

// Malicious USDC contract that exploits fee accounting
contract MaliciousUSDC {
OrderBook orderBook;
uint256 targetOrderId;
bool attacking;
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (!attacking && to == address(orderBook)) {
attacking = true;
// Re-enter during fee collection, before totalFees is updated
orderBook.buyOrder(targetOrderId);
// totalFees accounting could be manipulated
}
return true;
}
}
​
// Attack scenario:
// 1. Attacker deploys malicious USDC-like contract
// 2. If this malicious contract becomes the USDC reference (unlikely but theoretically possible)
// 3. During buyOrder(), malicious transferFrom re-enters the function
// 4. totalFees is incremented multiple times or manipulated
// 5. Protocol fee accounting becomes inconsistent
​
// Alternative scenario with malicious sell token:
contract MaliciousToken {
function transfer(address to, uint256 amount) external returns (bool) {
if (to != address(orderBook)) {
// Re-enter during token delivery to buyer
orderBook.buyOrder(anotherOrderId);
}
return true;
}
}

Recommended Mitigation

Follow the Checks-Effects-Interactions pattern by moving all state updates before external calls.
Alternatively, add a reentrancy guard.
​
​
Additional security consideration: Consider adding a self-purchase prevention check:
​
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
+ if (order.seller == msg.sender) revert CannotBuyOwnOrder();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
// ... rest of implementation
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
11 days ago
yeahchibyke Lead Judge 11 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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