Root + Impact
Description
In the OrderBook::getOrderDetailsString function, the tokenSymbol string variable is intended to represent the name of the token being sold. However, this currently only works for core tokens such as wBTC, wETH, and wSOL.
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";
}
string memory status = order.isActive
? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
: "Inactive (Filled/Cancelled)";
if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
status = "Expired (Awaiting Cancellation)";
} else if (!order.isActive) {
status = "Inactive (Filled/Cancelled)";
} else {
status = "Active";
}
details = string(
abi.encodePacked(
"Order ID: ",
order.id.toString(),
"\n",
"Seller: ",
Strings.toHexString(uint160(order.seller), 20),
"\n",
"Selling: ",
order.amountToSell.toString(),
" ",
tokenSymbol,
"\n",
"Asking Price: ",
order.priceInUSDC.toString(),
" USDC\n",
"Deadline Timestamp: ",
order.deadlineTimestamp.toString(),
"\n",
"Status: ",
status
)
);
return details;
}
Risk
Likelihood:
Occurrence will happened when the contract owner adds a new token address using the OrderBook::setAllowedSellToken function and the seller set their order of the non-core token
Impact:
OrderBook::getOrderDetailsString function will fail to provide the correct token name, resulting in incomplete or inaccurate order details. The more non-core tokens added, the harder it is to know witch token is witch
Proof of Concept
Owner set a newly allowed non-core token with OrderBook::setAllowedSellToken function
random person sell the new tokens
buyer looked up for the non-core token's details using OrderBook::getOrderDetailsString function and wouldn't know the name of the token.
PoC
[Step 1] : Create MockWMCO.sol file inside test/mocks/ folder and add the following:
pragma solidity 0.8.26;
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockWMCO is ERC20 {
uint8 tokenDecimals;
constructor(uint8 _tokenDecimals) ERC20("MockWMockCoin", "mWMCO") {
tokenDecimals = _tokenDecimals;
}
function decimals() public view override returns (uint8) {
return tokenDecimals;
}
function mint(address to, uint256 value) public {
uint256 updateDecimals = uint256(tokenDecimals);
_mint(to, (value * 10 ** updateDecimals));
}
}
[Step 2] : Add the following to TestOrderBook.t.sol :
// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.0;
import {Test, console2} from "forge-std/Test.sol";
import {OrderBook} from "../src/OrderBook.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
import {MockWBTC} from "./mocks/MockWBTC.sol";
import {MockWETH} from "./mocks/MockWETH.sol";
import {MockWSOL} from "./mocks/MockWSOL.sol";
+ import {MockWMCO} from "./mocks/MockWMCO.sol";
contract TestOrderBook is Test {
OrderBook book;
MockUSDC usdc;
MockWBTC wbtc;
MockWETH weth;
MockWSOL wsol;
+ MockWMCO wmco;
address owner;
address alice;
address bob;
address clara;
address dan;
+ address randomDude;
uint256 mdd;
function setUp() public {
owner = makeAddr("protocol_owner");
alice = makeAddr("will_sell_wbtc_order");
bob = makeAddr("will_sell_weth_order");
clara = makeAddr("will_sell_wsol_order");
dan = makeAddr("will_buy_orders");
+ randomDude = makeAddr("will_sell_wmco_order");
usdc = new MockUSDC(6);
wbtc = new MockWBTC(8);
weth = new MockWETH(18);
wsol = new MockWSOL(18);
+ wmco = new MockWMCO(18);
vm.prank(owner);
book = new OrderBook(address(weth), address(wbtc), address(wsol), address(usdc), owner);
usdc.mint(dan, 200_000);
wbtc.mint(alice, 2);
weth.mint(bob, 2);
wsol.mint(clara, 2);
+ wmco.mint(randomDude, 2);
mdd = book.MAX_DEADLINE_DURATION();
}
...
[Step 3] : At the very bottom of TestOrderBook.t.sol add the following test function:
function test_orderDetailsIsInaccurateForNonCoreTokens() public {
vm.startPrank(owner);
book.setAllowedSellToken(address(wmco), true);
vm.stopPrank();
vm.startPrank(randomDude);
wmco.approve(address(book), 2e18);
uint256 randomDudeId = book.createSellOrder(address(wmco), 2e18, 5_000e6, 2 days);
vm.stopPrank();
assert(randomDudeId == 1);
assert(wmco.balanceOf(randomDude) == 0);
assert(wmco.balanceOf(address(book)) == 2e18);
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 2 days);
vm.stopPrank();
assert(aliceId == 2);
assert(wbtc.balanceOf(alice) == 0);
assert(wbtc.balanceOf(address(book)) == 2e8);
console2.log(book.getOrderDetailsString(1));
console2.log(book.getOrderDetailsString(2));
}
[Step 4] : Use foundry forge command to test the contract function
forge test --mt test_orderDetailsIsInaccurateForNonCoreTokens -vv
Recommended Mitigation
Add a new mapping mapping(address tokenAddress => string tokenName) tokenAddressToName, this would require to refactor some of the contract's function, and for the OrderBook::setAllowedSellToken function, add a new parameter to set the token names.
- function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
+ function setAllowedSellToken(address _token, bool _isAllowed, string memory _tokenName) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // Cannot allow null or USDC itself
allowedSellToken[_token] = _isAllowed; // q can disable the seller of the current allowed token?
+ tokenAddressToName[_token] = _tokenName;
emit TokenAllowed(_token, _isAllowed);
}
