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;
if (order.tokenToSell == address(iWETH)) {
tokenSymbol = "wETH";
} else if (order.tokenToSell == address(iWBTC)) {
tokenSymbol = "wBTC";
} else if (order.tokenToSell == address(iWSOL)) {
tokenSymbol = "wSOL";
}
details = string(
abi.encodePacked(
"Order ID: ",
order.id.toString(),
"\n",
"Seller: ",
Strings.toHexString(uint160(order.seller), 20),
"\n",
"Selling: ",
order.amountToSell.toString(),
" ",
tokenSymbol,
"\n",
)
);
}
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);
newToken.mint(alice, 100e18);
vm.startPrank(alice);
newToken.approve(address(book), 100e18);
uint256 orderId = book.createSellOrder(
address(newToken),
100e18,
5000e6,
2 days
);
vm.stopPrank();
string memory orderDetails = book.getOrderDetailsString(orderId);
console2.log("=== TOKEN SYMBOL HANDLING ISSUE ===");
console2.log("Order details for new token:");
console2.log(orderDetails);
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
}