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

Bypassing `OrderBook::emergencyWithdrawERC20` restrictions via `OrderBook::setAllowedSellToken` allows owner to withdraw open order tokens

0xarinaitwe

Bypassing OrderBook::emergencyWithdrawERC20 restrictions via OrderBook::setAllowedSellToken allows owner to withdraw open order tokens

Description

  • The function OrderBook::emergencyWithdrawERC20 is intended to prevent withdrawal of core tokens.

  • This logic fails to consider tokens dynamically added to the system via OrderBook:setAllowedSellToken function. When the owner sets a new ERC20 token as allowed and a user deposits it by creating a sell order, the owner can withdraw it via OrderBook:emergencyWithdrawERC20, even when the order is still active.

// A new sell token set to `true` via this function
@> function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // Cannot allow null or USDC itself
allowedSellToken[_token] = _isAllowed;
​
emit TokenAllowed(_token, _isAllowed);
}
// Can be withdrawn by the owner via this function without any restriction
@> 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 sets a new ERC20 token as allowed token and a user deposits it by creating a sell order

Impact:

  • The owner can withdraw the newly set token via OrderBook:emergencyWithdrawERC20, even when the order is still active.

Proof of Concept

This test demonstrates how the owner of the contract adds MockWSUI as an allowed sell token.
A user creates a sell order and deposits MockWSUI into the contract.
The owner then calls OrderBook::emergencyWithdrawERC20 and withdraws the token to another address.
The user's open order is now effectively broken, yet the test still passes, highlighting the silent failure.

function test_allow_sell_token_and_withdraw_by_owner() public {
// Owner sets new sell token
vm.startPrank(owner);
book.setAllowedSellToken(address(wsui), true);
vm.stopPrank();
​
// Bob creates sell order for the new token, wsui
vm.startPrank(bob);
wsui.mint(bob, 2e18);
wsui.approve(address(book), 2e18);
uint256 bobId = book.createSellOrder(address(wsui), 2e18, 100e6, 2 days);
vm.stopPrank();
​
// Owner withdraws it while the order is still open
address chance = makeAddr("chance");
vm.prank(owner);
book.emergencyWithdrawERC20(address(wsui), 2e18, chance);
​
// Assert balance of receiver
assertEq(wsui.balanceOf(chance), 2e18);
}

Recommended Mitigation

Add tracking for tokens ever used in orders

Then update the OrderBook::emergencyWithdrawERC20 function:

+ mapping(address => bool) public usedInOrders;
​
function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
...
+ usedInOrders[sellToken] = true;
...
}
​
+ // Block withdrawal if token was ever used in orders
​
+ if (usedInOrders[_tokenAddress]) {
+ revert("Cannot withdraw token used in any order");
+ }
Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

0xarinaitwe Submitter
4 months ago
yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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