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

CEI Pattern Violation in amendSellOrder() Enables Reentrancy Attacks

lefeveje

CEI Pattern Violation in amendSellOrder() Enables Reentrancy Attacks

Description

The amendSellOrder() function should safely modify an existing sell order by adjusting token amounts (transferring additional tokens to contract or returning excess tokens to seller) and updating order parameters atomically without risk of reentrancy.

Specific Issue

The function violates the Checks-Effects-Interactions (CEI) pattern by making external calls (safeTransferFrom/safeTransfer) before completing all state updates, creating a reentrancy vulnerability that allows malicious token contracts to re-enter the function during token transfers.

function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
Order storage order = orders[_orderId];
// ... validation checks ...
uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
IERC20 token = IERC20(order.tokenToSell);
​
// @> EXTERNAL CALLS BEFORE STATE UPDATES
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 AFTER EXTERNAL CALLS
order.amountToSell = _newAmountToSell;
order.priceInUSDC = _newPriceInUSDC;
order.deadlineTimestamp = newDeadlineTimestamp;
​
emit OrderAmended(_orderId, _newAmountToSell, _newPriceInUSDC, newDeadlineTimestamp);
}
​

Risk

Likelihood:

  • wETH, wBTC, and wSOL are established tokens with standard ERC20 implementations

  • These legitimate tokens are unlikely to contain malicious reentrancy code

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

Impact:

  • Malicious token contracts could re-enter during safeTransferFrom or safeTransfer

  • Attacker could manipulate order state before original amendments are applied

  • Could lead to inconsistent order state, double-spending of amendments, or fund manipulation

  • More complex than createSellOrder due to bidirectional token transfers

Proof of Concept

// Malicious token that triggers reentrancy during transfers
contract MaliciousToken {
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 while original amendment is in progress
orderBook.amendSellOrder(targetOrderId, 500, 2000, 172800);
}
return true;
}
function transfer(address to, uint256 amount) external returns (bool) {
if (!attacking && to != address(orderBook)) {
attacking = true;
// Re-enter during token return to seller
orderBook.amendSellOrder(targetOrderId, 1500, 3000, 259200);
}
return true;
}
}
​
// Attack scenario:
// 1. Attacker creates order with malicious token
// 2. Calls amendSellOrder() to increase amount (triggers transferFrom)
// 3. During transferFrom, malicious contract re-enters amendSellOrder()
// 4. Order state manipulated before original amendment completes
// 5. Similar attack possible when decreasing amount (triggers transfer)

Recommended Mitigation

Follow the Checks-Effects-Interactions pattern by moving all state updates before external calls.
Alternatively, add a reentrancy guard.
​
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

lefeveje Submitter
4 months ago
yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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