OrderBook

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

abi.encodePacked() with Dynamic Types in Hashing (Ambiguous Encoding → Hash Collision)

Author Revealed upon completion

Description:

The OrderBook contract uses abi.encodePacked(...) to concatenate multiple dynamic values into a single string (e.g., inside getOrderDetailsString). While not currently hashed, if this packed encoding were ever passed to a keccak256 hash (e.g., for generating unique IDs or keys in mappings), it would become vulnerable to hash collisions due to Solidity’s non-standard packed mode.

When dynamic types (like string or bytes) are used in abi.encodePacked, Solidity concatenates them without length-prefix padding, which allows different sets of inputs to result in the same hash.

Impact:

If the packed-encoded string is ever hashed (e.g., keccak256(abi.encodePacked(...))) and used for:

1- Order uniqueness constraints
2- Mapping keys
3- Digital signatures

…then it may allow collision-based overwriting or order hijacking, resulting in order spoofing, loss of funds, or incorrect attribution

Proof of Concept:

function test_abiEncodePackedHashCollision() public pure {
string memory a1 = "abc";
string memory b1 = "d";
string memory a2 = "ab";
string memory b2 = "cd";
// Vulnerable: abi.encodePacked leads to same hash
bytes32 hashPacked1 = keccak256(abi.encodePacked(a1, b1));
bytes32 hashPacked2 = keccak256(abi.encodePacked(a2, b2));
assertEq(hashPacked1, hashPacked2, "Collision should occur with abi.encodePacked");
// Safe: abi.encode differentiates the inputs
bytes32 hashEncoded1 = keccak256(abi.encode(a1, b1));
bytes32 hashEncoded2 = keccak256(abi.encode(a2, b2));
assertTrue(hashEncoded1 != hashEncoded2, "No collision should occur with abi.encode");
}

Recommended Mitigation:

Avoid using abi.encodePacked(...) with multiple dynamic types if the result is passed into keccak256 or used for uniqueness/sensitive logic.

  1. Use abi.encode(...) instead — it includes padding and avoids ambiguity.

  2. use bytes.concat(...) if working purely with bytes or string.

  3. If you're only converting to a string for human display (e.g., getOrderDetailsString()), abi.encodePacked(...) is fine — but document clearly that it must never be reused in hash-based logic.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 22 hours ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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