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

`abi.encodePacked()` Hash Collision

nagatejakachapuram

Description

Specific Issue: The code uses abi.encodePacked() to construct a details string for informational purposes. While this string is not directly used in a hash function in the provided context, abi.encodePacked() concatenates dynamic data types (like strings and toString() outputs) without padding. This practice is inherently unsafe for any scenario where the concatenated output might later be used for hashing or unique identification, as different input combinations can result in identical packed outputs, leading to potential hash collisions.

Risk

Low

Root Cause: The use of abi.encodePacked() to concatenate dynamic types for the details string in src/OrderBook.sol.

Solidity

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
)
);
​

Likelihood: Low

Impact: Low

  • Impact 1: If the details string's usage changes in the future, and it is subsequently used as input for a hash function in a security-sensitive context (e.g., to generate a unique order hash, or for signature verification), then hash collisions could lead to severe logical errors, replay attacks, or data manipulation.

  • Impact 2: While not a direct exploit in its current form, it represents a "code smell" or bad practice. It deviates from recommended secure coding patterns for abi.encodePacked() and could lead to confusion or subtle vulnerabilities if similar patterns are replicated in contexts where data integrity is paramount.


Proof of Concept


This PoC demonstrates how abi.encodePacked() can lead to hash collisions due to a lack of padding, compared to abi.encode().

Solidity

pragma solidity ^0.8.0;
​
contract AbiEncodePackedCollision {
​
// Demonstrates how different inputs can result in the same abi.encodePacked hash
function demonstrateCollisionPacked() public pure returns (bytes, bytes, bool) {
// Example 1: Concatenating two 2-byte values
bytes data1 = abi.encodePacked(uint16(0x1234), uint16(0x5678)); // Result: 0x12345678
​
// Example 2: Encoding a single 4-byte value
bytes data2 = abi.encodePacked(uint32(0x12345678)); // Result: 0x12345678
​
// The keccak256 hash will be identical, indicating a collision risk
return (data1, data2, keccak256(data1) == keccak256(data2)); // This returns (0x12345678, 0x12345678, true)
}
​
// Demonstrates that abi.encode avoids collisions by padding
function demonstrateNoCollisionEncoded() public pure returns (bytes, bytes, bool) {
// Example 1: Encoding two 2-byte values with padding
bytes data1 = abi.encode(uint16(0x1234), uint16(0x5678)); // Result: 0x0...12340...5678 (32 bytes for each uint16)
​
// Example 2: Encoding a single 4-byte value with padding
bytes data2 = abi.encode(uint32(0x12345678)); // Result: 0x0...12345678 (32 bytes for the uint32)
​
// The keccak256 hash will be different, as expected
return (data1, data2, keccak256(data1) == keccak256(data2)); // This returns (padded_data1, padded_data2, false)
}
}
​

Recommended Mitigation


While the direct impact is low for display-only purposes, it's best practice to avoid abi.encodePacked() with dynamic types due to its known collision risks if the output is ever used in a hashing context. For concatenating strings for display, string.concat() (available in Solidity 0.8.12+) is the most idiomatic and clear approach without introducing padding.

Diff

- 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
- )
- );
+ // Using string.concat() for readable string concatenation (Solidity 0.8.12+)
+ details = string.concat(
+ "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
+ );
​

Reference Files

  • src/OrderBook.sol

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.