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

Failure to update order state in `OrderBook::emergencyWithdrawERC20` causing stale orders to arise.

mustby

Failure to update order state after withdrawal in OrderBook::emergencyWithdrawERC20

Description

When a user calls OrderBook::emergencyWithdrawERC20, the function is not updating the state/status of the order. Thus, it will create confusion for users on which order is active or not. This will affect all orders that are withdrawn through the OrderBook::emergencyWithdrawERC20 function.

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();
}
@> // state should be updated here
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
​
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}

Risk

Likelihood: High

  • Issues will arise anytime that the emergency withdrawal function is called.

Impact: Medium

  • Breaks trust and usability.

Proof of Concept

Input this unit test in TestOrderBook.t.sol. I also had to create a new "MockERC20" in the "test/mocks" folder to showcase this vulnerability using a non-core token.

function test_orderStatusNotChangedAfterEmergencyWithdrawal() public {
// Need to deploy another mock ("non-core") token to be used with emergency withdrawal
MockERC20 mockToken = new MockERC20(18); // identify the decimals to use
​
// Set MockERC20 as an allowed token by calling setAllowedToken on the OrderBook contract
vm.prank(owner);
book.setAllowedSellToken(address(mockToken), true);
vm.stopPrank();
​
// Create sell order with new token
vm.startPrank(alice);
mockToken.mint(alice, 1000);
mockToken.approve(address(book), 1000e18);
uint256 aliceId = book.createSellOrder(address(mockToken), 1000e18, 500e6, 2 days);
vm.stopPrank();
​
// Wait a day
vm.warp(block.timestamp + 1 days);
​
// Check user balance of new token
assert(mockToken.balanceOf(alice) == 0); // Alice should have no tokens left after creating the order
console2.log("Alice's Mock Token balance after creating order:", mockToken.balanceOf(alice));
​
// Emergency withdrawal of the new token
vm.prank(owner);
book.emergencyWithdrawERC20(address(mockToken), 1000e18, address(alice));
assert(mockToken.balanceOf(alice) == 1000e18); // Alice should have her tokens back after emergency withdrawal
assert(mockToken.balanceOf(address(book)) == 0); // OrderBook should have no tokens left after emergency withdrawal
console2.log("Alice's Mock Token balance after emergency withdrawal:", mockToken.balanceOf(alice));
​
// Check that the order is still available for purchase
string memory orderDetails = book.getOrderDetailsString(aliceId);
console2.log("Order details after emergency withdrawal:", orderDetails);
}

Here are the output logs showing that order status is still active after withdrawal:

Ran 1 test for test/TestOrderBook.t.sol:TestOrderBook
[PASS] test_orderStatusNotChangedAfterEmergencyWithdrawal() (gas: 1273338)
Logs:
Alice's Mock Token balance after creating order: 0
Alice's Mock Token balance after emergency withdrawal: 1000000000000000000000
Order details after emergency withdrawal: Order ID: 1
Seller: 0xaf6db259343d020e372f4ab69cad536aaf79d0ac
Selling: 1000000000000000000000
Asking Price: 500000000 USDC
Deadline Timestamp: 172801
Status: Active

Recommended Mitigation

Add two lines below. The order line to keep track of which order is cancelled. The "order.isActive" line to update state to false/inactive after the emergency withdrawal.

function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
+ Order storage order = orders[_orderId];
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();
}
​
+ order.isActive = false;
​
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
​
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
10 days ago
yeahchibyke Lead Judge 10 days ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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