OrderBook

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

Total fees variable is reset to zero after an external contract interaction against CEI best practice

Description

The total fees accrued by the protocol can be withdrawn using the withdrawFees function where it transfer the totalFees from the orderbook contract to a to adddress provided by the onlyOwner. However, in the withdrawFeess function, the totalFees is reset to zero after the external safeTransfer interaction which is contrary to the Checks Effects Interactions principle.

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);
}

Recommended Mitigation

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) {
revert InvalidAmount();
}
if (_to == address(0)) {
revert InvalidAddress();
}
uint256 feesToWithdraw = totalFees;
totalFees = 0;
iUSDC.safeTransfer(_to, feesToWithdraw);
- totalFees = 0;
emit FeesWithdrawn(_to);
}
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.