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

emergencyWithdrawERC20 Protection Gap

lefeveje

Incomplete Protection in emergencyWithdrawERC20() Allows Drainage of User-Deposited Tokens

Description

The emergencyWithdrawERC20() function should only allow withdrawal of accidentally sent tokens that are not part of the core protocol functionality, protecting all tokens that users have legitimately deposited for active trading orders.

Specific Issue

The function only protects the four hardcoded immutable tokens (wETH, wBTC, wSOL, USDC) but fails to protect tokens in the dynamic allowedSellToken mapping. This allows the owner to drain any whitelisted tokens that users have deposited for sell orders, even when those tokens are actively being used in the protocol.

function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
// @> ONLY PROTECTS HARDCODED IMMUTABLE TOKENS
if (
_tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) ||
_tokenAddress == address(iWSOL) || _tokenAddress == address(iUSDC)
) {
revert("Cannot withdraw core order book tokens via emergency function");
}
if (_to == address(0)) {
revert InvalidAddress();
}
// @> NO CHECK FOR DYNAMICALLY WHITELISTED TOKENS
// Missing: if (allowedSellToken[_tokenAddress]) revert("Cannot withdraw whitelisted tokens");
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
​
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}

The protection logic creates a mismatch:

  • Design: Contract supports dynamic token addition via setAllowedSellToken()

  • Protection: Only protects 4 specific hardcoded tokens

  • Gap: Whitelisted tokens used in active orders can be drained


Risk

Likelihood:

  • Owner has direct control over both setAllowedSellToken() and emergencyWithdrawERC20()

  • Attack requires only standard owner privileges, no special conditions

  • Could happen accidentally or maliciously

  • High likelihood because it requires no external dependencies

Impact:

  • Complete loss of user funds for any newly whitelisted tokens

  • Orders become unfulfillable, breaking protocol functionality

  • Users lose deposited tokens with no recovery mechanism

  • Destroys trust in protocol security

Proof of Concept

// Step-by-step attack scenario:
​
// Day 1: Owner adds support for LINK token
orderBook.setAllowedSellToken(LINK_ADDRESS, true);
​
// Days 2-30: Users create sell orders with LINK
// User A: createSellOrder(LINK_ADDRESS, 1000, 5000, 86400)
// User B: createSellOrder(LINK_ADDRESS, 2000, 8000, 86400)
// User C: createSellOrder(LINK_ADDRESS, 1500, 6000, 86400)
// Total: 4500 LINK tokens now locked in contract
​
// Day 31: Owner "emergency withdraws" all LINK tokens
orderBook.emergencyWithdrawERC20(LINK_ADDRESS, 4500, ownerWallet);
​
// Result:
// ✅ Transaction succeeds (LINK != wETH/wBTC/wSOL/USDC)
// ❌ Owner receives 4500 LINK tokens
// ❌ Users A, B, C have worthless orders that can never be fulfilled
// ❌ buyOrder() calls will fail: "Insufficient token balance"
​
// Proof of concept test:
contract ProofOfConcept {
function testEmergencyWithdrawWhitelistedToken() public {
// Setup
address LINK = address(new MockERC20("LINK", "LINK"));
orderBook.setAllowedSellToken(LINK, true);
// User deposits tokens via createSellOrder
vm.prank(user);
MockERC20(LINK).approve(address(orderBook), 1000);
vm.prank(user);
uint256 orderId = orderBook.createSellOrder(LINK, 1000, 5000, 86400);
// Verify tokens are in contract
assertEq(MockERC20(LINK).balanceOf(address(orderBook)), 1000);
// Owner drains the tokens
vm.prank(owner);
orderBook.emergencyWithdrawERC20(LINK, 1000, owner);
// Verify drainage
assertEq(MockERC20(LINK).balanceOf(address(orderBook)), 0);
assertEq(MockERC20(LINK).balanceOf(owner), 1000);
// Order becomes unfulfillable
vm.prank(buyer);
vm.expectRevert(); // Will fail due to insufficient token balance
orderBook.buyOrder(orderId);
}
}

Recommended Mitigation

Add protection for all whitelisted tokens, not just hardcoded ones.
​
Alternative approach - More restrictive emergency function.
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge
10 days ago
yeahchibyke Lead Judge 9 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

lefeveje Submitter
9 days ago
yeahchibyke Lead Judge
9 days ago
yeahchibyke Lead Judge 5 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.