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

Checks-Effects-Interactions Pattern Violation In the `buyOrder` function

mukulkolpe

Checks-Effects-Interactions Pattern Violation In the buyOrder function

Description

The buyOrder function violates CEI by performing external token transfers after updating critical state, creating a window for potential reentrancy exploitation if validation mechanisms fail

function 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 - Critical state updated early
order.isActive = false; // @> State change before external interactions
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
​
// INTERACTIONS - External calls create reentrancy opportunities
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
​
// EFFECTS - State changes after interactions
totalFees += protocolFee; // @> State update after external calls
​
emit OrderFilled(_orderId, msg.sender, order.seller);
}

Risk

Likelihood: Low

  • Currently mitigated by isActive validation check, but creates fragile security dependency

  • Future code modifications could inadvertently weaken validation logic

  • Non-standard pattern increases audit complexity and potential for overlooked vulnerabilities

  • Custom ERC20 tokens with complex transfer logic could exploit the pattern violation

Impact: Medium

  • If validation fails or is bypassed, multiple order executions could drain buyer funds

  • totalFees variable corruption could lead to accounting inconsistencies

  • Breaking of core protocol invariants (one order = one execution)

  • Potential for sophisticated attacks combining multiple vectors

Updates

Lead Judging Commences

yeahchibyke Lead Judge 10 days ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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