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

Missing Token Symbol for Non-Core Tokens in getOrderDetailsString

superdevfavour

The OrderBook:getOrderDetailsString function only handles symbol mapping for core tokens (wETH, wBTC, wSOL) but fails to handle non-core tokens, leading to poor UX.

Description

The function only maps symbols for the three core tokens (wETH, wBTC, wSOL) but the contract supports additional tokens through the OrderBook:setAllowedSellToken function.
When a non-core token is used in an order, the tokenSymbol variable remains uninitialized (empty string), resulting in incomplete order details.
Check the OrderBook:gerOrderDetailsString

@> string memory tokenSymbol;
if (order.tokenToSell == address(iWETH)) {
tokenSymbol = "wETH";
} else if (order.tokenToSell == address(iWBTC)) {
tokenSymbol = "wBTC";
} else if (order.tokenToSell == address(iWSOL)) {
tokenSymbol = "wSOL";
}
​

Risk

Likelihood: MEDIUM

  • Depends on whether non-core tokens are actually used

  • Owner can add any ERC20 token via OrderBook:setAllowedSellToken

  • Likely to occur if the protocol expands to support more tokens

Impact: LOW

  • User Experience: Poor readability of order details for non-core tokens

  • Information Loss: Users can't identify which token is being sold

  • Inconsistency: Different display quality for different token types

  • Functionality: Core functionality still works, only display is affected

Proof of Concept

The following test demonstrates the issue: when a non-core token (i.e., not wETH, wBTC, or wSOL) is used in an order, the getOrderDetailsString function fails to display the token’s symbol, resulting in incomplete and potentially confusing order information for users.

Explanation:

  • The test deploys a mock ERC20 token with a custom symbol (e.g., "mERC20").

  • The token is allowed for trading via setAllowedSellToken.

  • An order is created using this custom token.

  • When retrieving the order details string, the output is missing the token symbol after the amount, because the function only recognizes core tokens.

  • The test asserts that the symbol "mERC20" does not appear in the details, and that the output contains the amount followed by a blank space, confirming the bug.

Why this matters:

This proof of concept shows that users will not be able to distinguish which token is being sold if it is not one of the hardcoded core tokens. This can lead to confusion, mistakes in trading, and a poor user experience, especially as the protocol expands to support more tokens.
You can add these codes to the test file:

function testMissingTokenSymbol() public {
MockERC20 mockErc20 = new MockERC20(18);
​
vm.startPrank(owner);
book.setAllowedSellToken(address(mockErc20), true);
vm.stopPrank();
​
mockErc20.mint(dan, 2e18);
vm.startPrank(dan);
mockErc20.approve(address(book), 1e18);
uint256 danId = book.createSellOrder(
address(mockErc20),
1e18,
1820e6,
2 days
);
vm.stopPrank();
​
// Get order details string
string memory details = book.getOrderDetailsString(danId);
​
// Check that the symbol is missing (empty string)
// The output will be: "Selling: 1000000000000000000 " (notice the space after the amount)
assertTrue(
!contains(details, "mERC20"),
"Custom token symbol should not appear in details"
);
assertTrue(
contains(details, "Selling: 1000000000000000000 "),
"Details should show amount with empty symbol"
);
}
​
// Helper function to check if string contains substring
function contains(
string memory str,
string memory searchStr
) internal pure returns (bool) {
bytes memory strBytes = bytes(str);
bytes memory searchBytes = bytes(searchStr);
​
if (searchBytes.length > strBytes.length) {
return false;
}
​
for (uint i = 0; i <= strBytes.length - searchBytes.length; i++) {
bool found = true;
for (uint j = 0; j < searchBytes.length; j++) {
if (strBytes[i + j] != searchBytes[j]) {
found = false;
break;
}
}
if (found) {
return true;
}
}
return false;
}

Recommended Mitigation

Import the openzeppelinERC20 standard and check for the symbol instead of checking individual address with conditionals

+ import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
​
- string memory tokenSymbol;
- if (order.tokenToSell == address(iWETH)) {
- tokenSymbol = "wETH";
- } else if (order.tokenToSell == address(iWBTC)) {
- tokenSymbol = "wBTC";
- } else if (order.tokenToSell == address(iWSOL)) {
- tokenSymbol = "wSOL";
- }
+ string memory tokenSymbol = ERC20(order.tokenToSell).symbol();
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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