6

I'm searching for vulnrabilities and I have the following code:

function hash(
    address[] memory addresses,
    uint256[] memory values,
    bytes[] memory data,
    bytes32 description
) public returns (uint256) {
    return uint256(keccak256(abi.encode(targets, values, data, description)));
}

Can the function receive different inputs and return the same value? If yes, is there an example? If not, why not, and how is abi.encode handle these types?

I am aware that keccak256 is secure but if abi.encode is not secure than the keccack256 safety is poitless.

4
  • Does this answer your question? Handling keccak collisions in decoupled storage pattern Commented Nov 9, 2021 at 13:07
  • The unique hash comes from the keccak256 function, not abi.encode. Your question, reworded, is "is there any known collision in keccak256?" (cf the link supra for answer / TLDR: no) Commented Nov 9, 2021 at 13:08
  • @DrGorilla.eth No, my question is in the abi.encode. I know the keccak256 is secure, but if the abi.encode can return the same value than obviously the keccack256 will return the same value too.
    – egjlmn1
    Commented Nov 9, 2021 at 13:15
  • Fair enough, on top of @hrousille answer, you might check ethers github, iirc, this collision/type confusion risk is why they/he chose to enforce stricter types than web3 (docs.ethers.io/v5/api/utils/abi/coder) Commented Nov 9, 2021 at 20:38

1 Answer 1

5

It depends on what you mean by 'different values'.

But essentially, yes, abi.encode can return the same encoding when given different sets of parameters:

function collision() public pure {
    require(keccak256(abi.encode([1, 2], 3)) == keccak256(abi.encode(1, 2, 3)));
}

It can get a bit harder with dynamic parameters (variable length arrays) but not impossible neither :

function collision() public pure {
    bytes32 hash1 = keccak256(abi.encode(32, 1, 0x6100000000000000000000000000000000000000000000000000000000000000));
    bytes32 hash2 = keccak256(abi.encode("a"));
    require(hash1 == hash2);
}

Now whether that's actually usable is another question... the decode step requires a signature to interpret the encoding, and even if you used a different set of values to generate the same encoding, with a unique signature on the receiving end there will be no difference...

For example :

function wantsAnA(string memory _a) public pure {
    require (keccak256(abi.encodePacked(_a)) == keccak256(abi.encodePacked("a")), "Not a match");    
}

function weirdCalls() public {
    
    bytes memory encoded_1 = abi.encodeWithSignature("wantsAnA(string)", 32, 1, 0x6100000000000000000000000000000000000000000000000000000000000000);
    bytes memory encoded_2 = abi.encodeWithSignature("wantsAnA(string)", "a");
    
    (bool success1, bytes memory rvalue1) = address(this).call(encoded_1);
    (bool success2, bytes memory rvalue2) = address(this).call(encoded_2);
    
    require(success1 == true, "first call failed");
    require(success2 == true, "second call failed");
}

does not revert, even though the value given to the encoder are different, the output is the same so the interpretation on the callee's side is also the same.

Both encoding are interpreted as "a" by the function wantsAnA because it is explicitely looking for an encoding compatible with its signature : a string.

So the key takeaway is that if abi.encode generate the same output, it's the same encoding... so at a binary level there is no difference at all. So for your specific example, it's as if the "true" values were given as parameter, this is not a vulnerability, just a very contrived way of doing the same thing.


EDIT

If as you say you must respect input types, there will be no collision. You only deal with dynamic arrays, the encoding for those are :

  • 1st word : offset for start of data
  • 2dn word : length of data
  • 3rd word : data[0]
  • 4th word : data[1]
  • ...

For example : the string "a" from the previous examples is encoded as :

offset : 0x0000000000000000000000000000000000000000000000000000000000000020

length : 0x0000000000000000000000000000000000000000000000000000000000000001

data[0] : 0x6100000000000000000000000000000000000000000000000000000000000000

Everything is padded to 32 bytes and changing anything, length or data, will change the final output in the respective words. Static types are also immune to collision with different values (as long as there is padding and input types are respected).

By strictly respecting the same input types there is no collision possible with abi.encode and different input values. If they are different, they differ in actual data or in length/data either way this will be reflected in the encoding.

6
  • I have requirements on my input, in the function above in my question, the 3 arrays need to be the same length and have at least one element. Do you see a possibility of two different inputs that will result in the same output?
    – egjlmn1
    Commented Nov 10, 2021 at 9:02
  • just looked at my code again and the restriction is a bit different. The first input must have the same array sizes > 1, but the second input I want to attack with may be any length and even 0. Can this be exploited?
    – egjlmn1
    Commented Nov 10, 2021 at 9:59
  • does my edit answers your question with fixed types ?
    – hroussille
    Commented Nov 10, 2021 at 10:29
  • there is no collision even when I can have the arrays in any length I want? No possible arrangement of (offset, length, data) that will cause it?
    – egjlmn1
    Commented Nov 10, 2021 at 11:04
  • No, abi.encodePacked is vulnerable to collisions with multiple variable length arguments but abi.encode is not. keccak256(abi.encode(data, ...)) is the safe way to generate a unique ID for a given set of inputs.
    – hroussille
    Commented Nov 10, 2021 at 11:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.