OrderBook

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

Token can be disabled via `OrderBook::setAllowedSellToken` function while orders are active causing user inconvenience.

Token can be disabled via OrderBook::setAllowedSellToken function while orders are active causing user inconvenience.

Description

  • The OrderBook::setAllowedSellToken function allows the owner to disable a sell token at any time, without checking whether users currently have open orders involving that token.

  • This introduces a scenario where the owner can disable a market sell token while users still have open orders

@> 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);
}

Risk

Likelihood:

  • This happens when owner disables a sell token via the OrderBook::setAllowedSellToken function.

Impact:

  • Disabling a token while active orders exist causes the token to disappear from the frontend when it is excluded based on allowedSellToken[_token] status. This results in open orders becoming inaccessible through the user interface, leading to user confusion and interrupted trading activity.

Proof of Concept

Add this test to TestOrderBook.t.sol

It demonstrates that the contract owner can disable a sell token while active orders are still open

function test_disable_selling_token() public {
// Bob creates sell order for weth
vm.startPrank(bob);
weth.approve(address(book), 2e18);
uint256 bobId = book.createSellOrder(address(weth), 2e18, 100e6, 2 days);
vm.stopPrank();
// Owner sets weth to non allowed token
vm.startPrank(owner);
book.setAllowedSellToken(address(weth), false);
vm.stopPrank();
console.log("Owner disabled `weth` token while Bob's sell order is open");
}

Recommended Mitigation

Track the number of active orders per token directly during order creation and cancellation.

Then disallow disabling of sell token while orders are still open in the OrderBook::setAllowedSellToken function

+ mapping(address => uint256) public activeOrderCountByToken;
function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC, // n dont we need calculation mechanism for `_priceInUSDC`
uint256 _deadlineDuration
) public returns (uint256) {
...
+ // Increment active order count
+ activeOrderCountByToken[_tokenToSell]++;
...
}
function cancelSellOrder(uint256 _orderId) public {
...
+ activeOrderCountByToken[order.tokenToSell]--;
...
}
function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // Cannot allow null or USDC itself
+ // Diasble tokens with active orders
+ if (!_isAllowed && activeOrderCountByToken[_token] > 0) {
+ revert("Cannot disable token with active orders");
+ }
allowedSellToken[_token] = _isAllowed;
emit TokenAllowed(_token, _isAllowed);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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