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

Hash Collision Vulnerability in `getOrderDetailsString` Function

vicent00

Hash Collision Vulnerability in getOrderDetailsString Function

Description

The getOrderDetailsString function uses abi.encodePacked() to concatenate multiple strings and dynamic values, which can lead to hash collisions when different input combinations produce the same encoded output. This vulnerability occurs because abi.encodePacked() does not include length prefixes for dynamic types, allowing different string combinations to produce identical hashes.

The function concatenates order details including dynamic strings and numeric values without proper encoding, creating potential collision scenarios that could affect the integrity of order data representation.

// Root cause in the codebase with @> marks to highlight the relevant section
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)";
// @> PROBLEM: abi.encodePacked() with dynamic types can cause hash collisions
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
)
);
}

Risk

Likelihood:

  • Low - Hash collisions require very specific string combinations and are unlikely to occur with real order data

  • Low - The function is view-only and used only for data representation, not critical transactions

Impact:

  • Data integrity issues where different orders could produce identical string representations

  • Potential confusion in order management and user interface

  • No direct loss of funds as this is a view function

Proof of Concept

This PoC demonstrates how different order data can produce the same hash when using abi.encodePacked(). The vulnerability occurs because abi.encodePacked() concatenates strings without length prefixes, allowing different combinations to produce identical outputs.

// Test demonstrating hash collision vulnerability
function test_hash_collision_vulnerability() public {
// Create two different orders with potentially colliding data
vm.startPrank(alice);
uint256 orderId1 = book.createSellOrder(address(wbtc), 1000, 50000, 1 days);
vm.stopPrank();
vm.startPrank(bob);
uint256 orderId2 = book.createSellOrder(address(weth), 2000, 25000, 2 days);
vm.stopPrank();
// Get string representations
string memory details1 = book.getOrderDetailsString(orderId1);
string memory details2 = book.getOrderDetailsString(orderId2);
// Demonstrate potential collision scenarios
// Example: Different combinations of strings can produce same hash
bytes memory encoded1 = abi.encodePacked(
"Order ID: ", "1",
"\n", "Seller: ", "0x1234567890123456789012345678901234567890",
"\n", "Selling: ", "1000", " ", "wBTC"
);
bytes memory encoded2 = abi.encodePacked(
"Order ID: ", "1",
"\n", "Seller: ", "0x1234567890123456789012345678901234567890",
"\n", "Selling: ", "100", "0", " ", "wBTC" // Different amount, same result
);
// These could potentially produce the same hash
bytes32 hash1 = keccak256(encoded1);
bytes32 hash2 = keccak256(encoded2);
// In some cases, different inputs produce same hash
// This demonstrates the collision vulnerability
}

Collision Scenarios:

  1. String concatenation without length prefixes - Different string combinations can produce identical hashes

  2. Numeric data mixed with strings - Order IDs, amounts, and prices can create collision patterns

  3. Dynamic status strings - Different order states can lead to hash collisions

  4. Address formatting - Hex string representations can cause collisions

Recommended Mitigation

Explanation: The fix uses abi.encode() instead of abi.encodePacked() to include proper length prefixes for dynamic types. This ensures that different input combinations will always produce unique hashes, preventing collision attacks.

  1. Use abi.encode() instead of abi.encodePacked() - Includes length prefixes for dynamic types

  2. Add input validation - Ensure order data is properly formatted

  3. Consider using structured data - Use JSON or other structured formats for complex data

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)";
​
- // Remove abi.encodePacked() which can cause hash collisions
- 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
- )
- );
​
+ // Use abi.encode() to prevent hash collisions
+ details = string(
+ abi.encode(
+ "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
+ )
+ );
}

Additional Recommendations:

  1. Add comprehensive testing for hash collision scenarios

  2. Consider using structured data formats like JSON for complex order representations

  3. Implement input validation to ensure order data integrity

  4. Add unit tests that verify unique hash generation for different order combinations

  5. Consider using deterministic encoding for order string generation

Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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