OrderBook

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

Misuse Of emergencyWithdrawERC20() Allows Draining of Allowed Tokens Except for Hardcoded List

Root + Impact

Description

  • The emergencyWithdrawERC20() function is designed to prevent withdrawal of core trading tokens, but it relies on hardcoded address checks rather than the dynamic allowedSellToken mapping that controls which tokens can be traded.

Current Hardcoded Protection:

if (
_tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) ||
_tokenAddress == address(iWSOL) || _tokenAddress == address(iUSDC)
) {
revert("Cannot withdraw core order book tokens via emergency function");
}

However, the contract allows the owner to dynamically add new trading tokens:

function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken();
allowedSellToken[_token] = _isAllowed;// @> New tokens marked as tradeable
emit TokenAllowed(_token, _isAllowed);
}

This creates an inconsistency where newly added trading tokens can still be withdrawn via the emergency function because they bypass the hardcoded checks, potentially draining tokens that users have locked in active orders.

function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
if (
_tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) || _tokenAddress == address(iWSOL)
|| _tokenAddress == address(iUSDC) // @> Only hardcoded tokens are protected
) {
revert("Cannot withdraw core order book tokens via emergency function");
}
// ... rest of function can withdraw any other token
}

Risk

Likelihood:

  • The owner expands the platform by adding new tokens via setAllowedSellToken() to increase trading pairs and attract more users

  • Users create sell orders with newly added tokens, depositing significant amounts into the contract for trading

Impact:

  • Direct theft of user funds deposited in sell orders for non-hardcoded tokens, with no recovery mechanism

  • Complete breakdown of order fulfillment system as orders become permanently unfillable when underlying tokens are drained

Proof of Concept

  1. Owner adds a new token as an allowed trading token: orderBook.setAllowedSellToken(wmaticAddress, true);

  2. Users create sell orders with the new token, locking tokens in the contract.

  3. Owner can withdraw all of the new token via emergency function since it's not hardcoded: orderBook.emergencyWithdrawERC20(wmaticAddress, amount, ownerWallet);

  4. User orders become unfillable as the contract no longer holds the tokens.

    // 1. Owner adds WMATIC as a new trading token
    orderBook.setAllowedSellToken(address(iWMATIC), true); // WMATIC
    // 2. Alice creates a sell order with 1000 WMATIC
    orderBook.createSellOrder(address(iWMATIC), 1000e18, 500e6, 1 days);
    // 3. Bob creates another sell order with 500 WMATIC
    orderBook.createSellOrder(address(iWMATIC), 500e18, 250e6, 2 days);
    // 4. Owner drains all WMATIC from contract (1500 total)
    orderBook.emergencyWithdrawERC20(
    address(iWMATIC),
    ownerWallet
    ); // ✅ Succeeds - WMATIC not in hardcoded list
    // 5. Alice and Bob's orders are now unfillable - their 1500 WMATIC is stolen
    // buyOrder() will fail with insufficient balance when trying to transfer tokens

Recommended Mitigation

The fix involves replacing the hardcoded token address checks with a dynamic check against the allowedSellToken mapping. This creates consistency between the token management system and the emergency withdrawal protection.

The current implementation creates a dangerous disconnect where:

  1. New tokens can be dynamically added for trading via setAllowedSellToken()

  2. But the emergency function only protects hardcoded tokens

  3. This leaves newly added trading tokens vulnerable to withdrawal despite having active user orders

By checking the allowedSellToken mapping instead, the emergency function will automatically protect any token that's currently enabled for trading, regardless of when it was added. This ensures that user funds locked in orders for any allowed trading token remain secure.

The mitigation also maintains the original intent of the emergency function - to recover accidentally sent tokens that aren't part of the trading system - while preventing the withdrawal of any token that users can actively deposit through sell orders.

function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
- if (
- _tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) || _tokenAddress == address(iWSOL)
- || _tokenAddress == address(iUSDC)
- ) {
- revert("Cannot withdraw core order book tokens via emergency function");
- }
+ // Prevent withdrawal of any token that's allowed for trading or is USDC
+ if (allowedSellToken[_tokenAddress] || _tokenAddress == address(iUSDC)) {
+ revert("Cannot withdraw active trading tokens via emergency function");
+ }
if (_to == address(0)) {
revert InvalidAddress();
}
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}
Updates

Lead Judging Commences

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

Support

FAQs

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