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

Race Condition on Fee Withdrawal - Fee loss

perun84

Root + Impact

Description

  • Normal behavior: The withdrawFees function allows the contract owner to withdraw accumulated protocol fees (totalFees) in USDC. The function transfers the current totalFees to the owner and then resets totalFees to zero.

  • Issue: If a user calls buyOrder (which increases totalFees) during the execution of withdrawFees - specifically, after the transfer but before totalFees is set to zero - the newly added fee will be overwritten and lost, making it impossible for the owner to withdraw it later.

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) {
revert InvalidAmount();
}
if (_to == address(0)) {
revert InvalidAddress();
}
​
iUSDC.safeTransfer(_to, totalFees); // @> transfer occurs here
totalFees = 0; // @> state is reset here (race condition)
emit FeesWithdrawn(_to);
}

Risk

Likelihood:

  • This will occur when a user calls buyOrder and pays a protocol fee at the same time as the owner is withdrawing fees.

  • The risk increases with higher contract usage and frequent fee withdrawals.

Impact:

  • Protocol fees paid during this race window will be lost and cannot be withdrawn by the owner.

  • This results in a direct loss of protocol revenue and may cause accounting discrepancies.

Proof of Concept

Call buyOrder after the transfer but before totalFees is set to zero fees of that buying will be lost.

// Owner calls withdrawFees (totalFees = 100)
// Before totalFees = 0, a user calls buyOrder (totalFees += 3, now totalFees = 103)
// withdrawFees sets totalFees = 0
// The 3 USDC fee is now lost and cannot be withdrawn

Recommended Mitigation

Get amount of fees and set fees to 0 before transfer.

- iUSDC.safeTransfer(_to, totalFees);
-
- totalFees = 0;
+ uint256 amount = totalFees;
+ totalFees = 0;
+ iUSDC.safeTransfer(_to, amount);
Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 1 month ago
yeahchibyke Lead Judge about 1 month 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.