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

H-01. Reentrancy Risk and CEI Pattern Violation

0xriz0

Root + Impact

  • Reentrancy Risk + CEI Pattern Violation

Description

  • Smart contracts should follow the Checks-Effects-Interactions (CEI) pattern to prevent reentrancy attacks by updating state before performing external calls.

  • Multiple functions in the OrderBook contract violate the CEI pattern by performing external token transfers before updating critical state variables, creating potential reentrancy vulnerabilities that could allow malicious actors to manipulate contract state.

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
// ... validation checks ...
uint256 orderId = _nextOrderId++;
// @> External call BEFORE state update - violates CEI pattern
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
// @> State update happens AFTER external call - reentrancy risk
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
amountToSell: _amountToSell,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
}
​
function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
// ... validation checks ...
// @> External token transfers BEFORE state updates - violates CEI pattern
if (_newAmountToSell > order.amountToSell) {
uint256 diff = _newAmountToSell - order.amountToSell;
token.safeTransferFrom(msg.sender, address(this), diff);
} else if (_newAmountToSell < order.amountToSell) {
uint256 diff = order.amountToSell - _newAmountToSell;
token.safeTransfer(order.seller, diff);
}
// @> State updates happen AFTER external calls - reentrancy risk
order.amountToSell = _newAmountToSell;
order.priceInUSDC = _newPriceInUSDC;
order.deadlineTimestamp = newDeadlineTimestamp;
}
​
function buyOrder(uint256 _orderId) public {
// ... validation checks ...
// @> State update happens early but external calls follow - partial CEI violation
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
// @> Multiple external calls - reentrancy risk
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
// @> State update AFTER external calls - violates CEI pattern
totalFees += protocolFee;
}

Risk

Likelihood:

  • Malicious ERC20 tokens with hooks in transfer functions can trigger reentrancy attacks during token transfers

  • Attackers can deploy malicious tokens that call back into the contract during safeTransferFrom or safeTransfer operations

Impact:

  • Attackers could create multiple orders with the same orderId by reentering createSellOrder before state is updated

  • Order state manipulation is possible in amendSellOrder through reentrancy during token transfers

  • Double-spending attacks could occur in buyOrder if reentrancy happens before totalFees is updated

  • Contract state inconsistency could lead to locked funds or incorrect order tracking

Proof of Concept

The CEI pattern violations create multiple attack vectors that can be exploited through malicious token contracts.

// When attacking createSellOrder Func
// Attacker can create multiple orders with overlapping orderIds
// causing state corruption and potential fund loss
​
// When attacking amendSellOrder Func
// Attacker can manipulate order state during amendment process
​
// When attacking buyOrder Func
// Attacker can potentially manipulate fee accounting or buy multiple orders

Recommended Mitigation

Implement the CEI pattern consistently across all functions by performing all state updates before external calls and add reentrancy protection

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
// ... validation checks ...
uint256 deadlineTimestamp = block.timestamp + _deadlineDuration;
uint256 orderId = _nextOrderId++;
​
- IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
-
- orders[orderId] = Order({
+ // Update state BEFORE external call (CEI pattern)
+ orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
amountToSell: _amountToSell,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
+
+ // External call AFTER state update
+ IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
​
emit OrderCreated(orderId, msg.sender, _tokenToSell, _amountToSell, _priceInUSDC, deadlineTimestamp);
return orderId;
}
​
function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
// ... validation checks ...
uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
IERC20 token = IERC20(order.tokenToSell);
​
+ // Update state BEFORE external calls (CEI pattern)
+ uint256 oldAmount = order.amountToSell;
+ order.amountToSell = _newAmountToSell;
+ order.priceInUSDC = _newPriceInUSDC;
+ order.deadlineTimestamp = newDeadlineTimestamp;
+
// Handle token amount changes
- if (_newAmountToSell > order.amountToSell) {
+ if (_newAmountToSell > oldAmount) {
// Increasing amount: Transfer additional tokens from seller
- uint256 diff = _newAmountToSell - order.amountToSell;
+ uint256 diff = _newAmountToSell - oldAmount;
token.safeTransferFrom(msg.sender, address(this), diff);
- } else if (_newAmountToSell < order.amountToSell) {
+ } else if (_newAmountToSell < oldAmount) {
// Decreasing amount: Transfer excess tokens back to seller
- uint256 diff = order.amountToSell - _newAmountToSell;
+ uint256 diff =
Updates

Lead Judging Commences

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

Support

FAQs

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