sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

Bauer - Merkle leaf values are 64 bytes before hashing which can lead to merkle tree collisions #106

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

Bauer

medium

Merkle leaf values are 64 bytes before hashing which can lead to merkle tree collisions

Summary

The protocol hashes 64 bytes when calculating leaf allowing it to collide with the internal nodes of the merkle tree.

Vulnerability Detail

MerkleProofUpgradeable.sol puts the following warning at the beginning of the contract:

 * WARNING: You should avoid using leaf values that are 64 bytes long prior to
 * hashing, or use a hash function other than keccak256 for hashing leaves.
 * This is because the concatenation of a sorted pair of internal nodes in
 * the merkle tree could be reinterpreted as a leaf value.

The following code has an issue because it uses input.blobRoot and input.bridgeRoot as the bases for leaves, both of which are bytes32 (totaling 64 bytes). This allows conflicts between leaves and inner nodes.

 if (
            !input.dataRootProof.verify(
                dataRootCommitment, input.dataRootIndex, keccak256(abi.encode(input.blobRoot, input.bridgeRoot))
            )
        ) {
            revert InvalidDataRootProof();
        }

Impact

Users can abuse Merkle Tree collisions

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L491

Tool used

Manual Review

Recommendation

Use @openzeppelin/merkle-tree instead.

Duplicate of #10

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { invalid and a duplicate of issue 061}