OrderBook

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

Token Symbol Handling Issue in `getOrderDetailsString`

Token Symbol Handling Issue in getOrderDetailsString

Description

  • The getOrderDetailsString function is designed to provide human-readable order information including token symbols for display purposes

  • The function uses hardcoded symbols for the three initial tokens (wETH, wBTC, wSOL) but returns empty strings for any new tokens added via setAllowedSellToken

function getOrderDetailsString(uint256 _orderId) public view returns (string memory details) {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
string memory tokenSymbol;
// @> Hardcoded token symbol mapping only for initial tokens
if (order.tokenToSell == address(iWETH)) {
tokenSymbol = "wETH";
} else if (order.tokenToSell == address(iWBTC)) {
tokenSymbol = "wBTC";
} else if (order.tokenToSell == address(iWSOL)) {
tokenSymbol = "wSOL";
}
// @> No handling for newly added tokens - tokenSymbol remains empty
// ... rest of function uses tokenSymbol in string concatenation
details = string(
abi.encodePacked(
"Order ID: ",
order.id.toString(),
"\n",
"Seller: ",
Strings.toHexString(uint160(order.seller), 20),
"\n",
"Selling: ",
order.amountToSell.toString(),
" ",
tokenSymbol, // @> Empty string for new tokens
"\n",
// ... rest of string formatting
)
);
}

Risk

Likelihood: Medium

  • Owner is likely to add new tokens via setAllowedSellToken as the protocol grows

  • Users and front-ends will call getOrderDetailsString for display purposes

  • No validation prevents the creation of orders with tokens that have empty symbols

Impact: Medium

  • Confusing user experience with incomplete order information

  • Front-end applications may display malformed order details

  • Potential integration issues with external systems expecting proper token symbols

Proof of Concept

contract MockNewToken {
string public name = "New Protocol Token";
string public symbol = "NPT";
uint8 public decimals = 18;
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
constructor() {
balanceOf[msg.sender] = 1000e18;
}
function transfer(address to, uint256 amount) external returns (bool) {
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
function approve(address spender, uint256 amount) external returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
}
}
function test_tokenSymbolHandlingIssue() public {
MockNewToken newToken = new MockNewToken();
vm.prank(owner);
book.setAllowedSellToken(address(newToken), true);
// Alice creates an order with the new token
newToken.mint(alice, 100e18);
vm.startPrank(alice);
newToken.approve(address(book), 100e18);
uint256 orderId = book.createSellOrder(
address(newToken),
100e18,
5000e6,
2 days
);
vm.stopPrank();
// Get order details string
string memory orderDetails = book.getOrderDetailsString(orderId);
console2.log("=== TOKEN SYMBOL HANDLING ISSUE ===");
console2.log("Order details for new token:");
console2.log(orderDetails);
// The string should show "100 NPT" but shows "100 " instead
vm.startPrank(bob);
weth.approve(address(book), 1e18);
uint256 wethOrderId = book.createSellOrder(
address(weth),
1e18,
2500e6,
2 days
);
vm.stopPrank();
string memory wethOrderDetails = book.getOrderDetailsString(
wethOrderId
);
console2.log("\n=== COMPARISON WITH CORE TOKEN ===");
console2.log("WETH order details (properly formatted):");
console2.log(wethOrderDetails);
assertTrue(
bytes(orderDetails).length > 0,
"Order details should not be empty"
);
vm.startPrank(dan);
usdc.approve(address(book), 5000e6);
book.buyOrder(orderId);
vm.stopPrank();
assertEq(
newToken.balanceOf(dan),
100e18,
"Dan should receive the new tokens"
);
}

Recommended Mitigation

+ import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
function getOrderDetailsString(uint256 _orderId) public view returns (string memory details) {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
+ string memory tokenSymbol = getTokenSymbol(order.tokenToSell);
- 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";
- }
// ... rest of function unchanged
}
+ function getTokenSymbol(address _token) public view returns (string memory) {
+ try IERC20Metadata(_token).symbol() returns (string memory symbol) {
+ return symbol;
+ } catch {
+ // Fallback to hardcoded symbols for tokens without metadata
+ if (_token == address(iWETH)) return "wETH";
+ if (_token == address(iWBTC)) return "wBTC";
+ if (_token == address(iWSOL)) return "wSOL";
+ return "UNKNOWN";
+ }
+ }

Alternative Solution with Symbol Registry:

+ mapping(address => string) public tokenSymbols;
+ function setTokenSymbol(address _token, string memory _symbol) external onlyOwner {
+ require(allowedSellToken[_token], "Token not allowed");
+ tokenSymbols[_token] = _symbol;
+ }
function getOrderDetailsString(uint256 _orderId) public view returns (string memory details) {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
+ string memory tokenSymbol = bytes(tokenSymbols[order.tokenToSell]).length > 0
+ ? tokenSymbols[order.tokenToSell]
+ : getTokenSymbol(order.tokenToSell);
// ... rest of function
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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