OrderBook

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

Improper Ordering of State Update in withdrawFees() Violates CEI Pattern

Root + Impact

Minor: No known exploit with current tokens, but violates CEI pattern

  • Could become Medium if used with untrusted ERC20s in future or if contract is forked

Description

In the withdrawFees() function, the contract performs an external call to iUSDC.safeTransfer() before updating the totalFees state variable to zero This violates the Checks-Effects-Interactions (CEI) pattern and introduces a theoretical risk window for reentrancy especially if USDC is ever replaced with a custom or malicious ERC20 token.

Even though USDC is generally safe and reentrancy is unlikely here, following CEI is a security best practice and protects against unexpected behavior in future token changes or proxy upgrades.

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) {
revert InvalidAmount();
}
if (_to == address(0)) {
revert InvalidAddress();
}
iUSDC.safeTransfer(_to, totalFees);
@> totalFees = 0;
emit FeesWithdrawn(_to);
}

Risk

Likelihood:

  • The external call could re-enter the contract

  • totalFees is still non-zero during that call

Impact:

  • Minor: No known exploit with current tokens, but violates CEI pattern

  • Could become Medium if used with untrusted ERC20s in future or if contract is forked

Proof of Concept

chances of reentrancy

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) revert InvalidAmount();
if (_to == address(0)) revert InvalidAddress();
iUSDC.safeTransfer(_to, totalFees); // ⚠️ External call first
totalFees = 0; // ⚠️ State updated *after*
}

Recommended Mitigation

Apply the CEI pattern by updating state before the external call:

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) revert InvalidAmount();
if (_to == address(0)) revert InvalidAddress();
uint256 amount = totalFees;
totalFees = 0;
iUSDC.safeTransfer(_to, amount);
emit FeesWithdrawn(_to);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
9 days ago
yeahchibyke Lead Judge 9 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

CEI pattern not followed in `withdrawFees()` function

`withdrawFees()` function performs an external transfer using `iUSDC.safeTransfer()` before resetting totalFees. This breaks the `Checks-Effects-Interactions (CEI)` pattern and can lead to incorrect internal state if the transfer fails for any reason.

Support

FAQs

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