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

Unrestricted emergency withdrawal of non-Core tokens

tanug

Unrestricted emergency withdrawal of non-Core tokens enables owner to exploit seller funds, undermining the security for participants listing non-core tokens

Description

  • The OrderBook::emergencyWithdrawERC20 function allows the contract owner to withdraw any token not explicitly blocked (i.e., not wETH, wBTC, wSOL, or USDC).

  • Order created using that token will enable owner to maliciously drain the seller’s funds from the contract.

// Root cause in the codebase with @> marks to highlight the relevant section
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");
}
if (_to == address(0)) {
revert InvalidAddress();
}
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
​
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}

Risk

Likelihood:

  • When the owner adds a new token as sellable and then maliciously withdraws a seller’s deposited tokens from the contract.

Impact:

  • Undermines the trust model and asset security for participants listing non-core tokens.

Proof of Concept

Owner can easily withdraw the non-core tokens in emergencies when orders are active for non-core tokens.

function test_EmergencyWithdrawal_Of_NonCore_Tokens() external {
TestToken token = new TestToken();
​
vm.prank(owner);
book.setAllowedSellToken(address(token), true);
​
vm.startPrank(alice);
token.mint(alice, 2e18);
token.approve(address(book), 2e18);
book.createSellOrder(address(token), 2e18, 3000e6, 3600);
vm.stopPrank();
​
assertEq(token.balanceOf(address(book)), 2e18);
​
vm.prank(owner);
vm.expectEmit();
emit OrderBook.EmergencyWithdrawal(address(token), 2e18, owner);
book.emergencyWithdrawERC20(address(token), 2e18, owner);
​
assertEq(token.balanceOf(address(book)), 0);
assertEq(token.balanceOf(owner), 2e18);
}

Recommended Mitigation

  1. Introduce a separate mapping for allowed tokens and only permit emergency withdrawal for tokens not used in active or historical orders.

  2. Make emergency withdrawal time-locked or permanently disabled once any order using a given token is created, ensuring full safety for user assets.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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