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

CEI Pattern Violation in createSellOrder() Enables Reentrancy Attacks

lefeveje

CEI Pattern Violation in createSellOrder() Enables Reentrancy Attacks

Description

Normal Behavior

The createSellOrder() function should safely create a new sell order by transferring tokens from the seller to the contract and storing the order details atomically without risk of reentrancy.

Specific Issue

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

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
// ... validation checks ...
uint256 deadlineTimestamp = block.timestamp + _deadlineDuration;
uint256 orderId = _nextOrderId++;
​
// @> EXTERNAL CALL BEFORE STATE COMPLETION
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
​
// @> STATE UPDATE AFTER EXTERNAL CALL
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
amountToSell: _amountToSell,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
​
emit OrderCreated(orderId, msg.sender, _tokenToSell, _amountToSell, _priceInUSDC, deadlineTimestamp);
return orderId;

Risk

Likelihood:

  • A malicious token contract must be whitelisted via setAllowedSellToken(),

  • Owner controls token whitelist, reducing immediate risk

  • However, owner could unknowingly whitelist a malicious token

  • Overall it is Low

Impact:

  • Malicious token contracts can re-enter during safeTransferFrom

  • Could potentially manipulate order state or create multiple orders with same tokens

  • May lead to inconsistent contract state or fund loss

Proof of Concept

// Malicious ERC20 token contract
contract MaliciousToken {
OrderBook orderBook;
address attacker;
bool attacking;
constructor(address _orderBook) {
orderBook = OrderBook(_orderBook);
attacker = msg.sender;
}
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
// Re-enter createSellOrder during token transfer
if (!attacking && to == address(orderBook)) {
attacking = true;
// Re-entrant call before original order is stored
orderBook.createSellOrder(address(this), amount, 1000, 86400);
}
return true;
}
// Other ERC20 functions...
}
​
// Attack scenario:
// 1. Owner unknowingly whitelists MaliciousToken
// 2. Attacker calls createSellOrder() with MaliciousToken
// 3. During safeTransferFrom, malicious contract re-enters
// 4. Multiple orders created or state manipulated before original call completes

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.