404Notfound - Merkle leaf values for `dataRootCommitment` are 64 bytes before hashing which can lead to merkle tree collisions

[sherlock-admin2]

404Notfound

medium

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

Summary

AvailBridge hashes 64 bytes when calculating leaf allowing it to collide with the internal nodes of the Merkle tree.

Vulnerability Detail

The hash value of 64 bytes of blobRoot and bridgeRoot is used as a leaf node in the function AvailBridge.sol#_checkDataRoot(). Both blobRoot and bridgeRoot passed by the user are uint256 (32 bytes each for 64 bytes total), while the parent data that is hashed is also 64 bytes. As a result, it is possible to have a hash collision between a leaf and any node in the tree. This allows for proofs to be repeated multiple times by taking subtrees as leaves.

    function _checkDataRoot(MerkleProofInput calldata input) private view {
        bytes32 dataRootCommitment = vectorx.dataRootCommitments(input.rangeHash);
        if (dataRootCommitment == 0x0) {
            revert DataRootCommitmentEmpty();
        }
        // we construct the data root here internally, it is not possible to create an invalid data root that is
        // also part of the commitment tree
        if (
            !input.dataRootProof.verify(
                dataRootCommitment, input.dataRootIndex, keccak256(abi.encode(input.blobRoot, input.bridgeRoot))
            )
        ) {
            revert InvalidDataRootProof();
        }
    }

Impact

The public/external function verifyBlobLeaf() and verifyBridgeLeaf() calling _checkDataRoot() may return true with invalid input, which may bring unexpected errors.

Code Snippet

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

Tool used

Manual Review

Recommendation

Use a combination of variables that don't sum to 64 bytes, for example, using encodePacked to combine blobRoot and bridgeRoot.

Duplicate of #10

[sherlock-admin]

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

takarez commented:

invalid because {invalid: also a duplicate of issue 061}