search

code-423n4 / 2022-10-blur-findings

1 stars 0 forks source link

Merkle verifier library verifies intermediate inputs #823

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/MerkleVerifier.sol#L8 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/MerkleVerifier.sol#L17 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/MerkleVerifier.sol#L33

Vulnerability details

Vulnerability details

Description

MerkleVerifier provides a set of functions for verification of a Merkle proof by performing an inclusion check of input against a binary tree. This is implemented as consecutively hashing concatenated sibling nodes until a root hash is generated. The input is one of the leaf hash values, while the proof is a path through the tree containing the missing hash information to regenerate the root.

The library allows usage of arbitrary length proof paths. This enables an issue with shorter paths to resolve to the same root. Hence, the known hash of an intermediate node is a valid input as well. Considering the leaf nodes with equal parent node h0 and h1, the hashed concatenation hash(h0 || h1) of those hashes would be a valid input along a shorter path. An attacker could utilize the known pre-image to prove its inclusion in the tree.

Considering it as a separate library, it is a critical vulnerability. However, the value passed to the library as a leaf hash is calculated by hashing the bytes, which size is higher than 64 bytes. All in all, exploiting it directly is impossible, but there is no explicit protection against it. Please note, neither in the comments nor in the documentation it wasn't indicated that it is a known attack surface.

Recommended Mitigation Steps

Add the tree height into the signed data, and make a check of equality againsе the Merkle proof length.

GalloDaSballo commented 2 years ago

I think this is missing a specific hash clash otherwise the odds of the clash have to be assumed at 2^256-1

blur-io-toad commented 2 years ago

I don't think the length check is necessary.

GalloDaSballo commented 2 years ago

Am not punishing the report, but kindly ask the Warden to add a specific instance next time as we cannot assume randomness is broken without proofs.

Closing as invalid in lack of evidence