sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

Tricko - `AvailBridge._checkDataRoot()` is susceptible to preimage attacks on merkle trees. #75

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

Tricko

medium

AvailBridge._checkDataRoot() is susceptible to preimage attacks on merkle trees.

Summary

The use of two 32 bytes user-controlled inputs (input.blobRoot and input.bridgeRoot) in AvailBridge._checkDataRoot() makes it susceptible to the second preimage attack. As a result, it is possible to use a combination of the internal nodes of the data commitment tree as inputs, making the non-leaf values to be incorrectly verified as valid leaves.

Vulnerability Detail

Examining the code snippet from AvailBridge._checkDataRoot() below, it utilizes abi.encode on input.blobRoot and input.bridgeRoot, followed by hashing and passing the result as a leaf for verification.

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();
    }
}

The second preimage attack on merkle trees is a well know issue when valid leaf values are 64 bytes. This enables attackers to use the concatenated hashes of two internal nodes of the merkle tree (32 bytes each) and pass this value to the merkle tree validation, leading to it being accepted as a valid leaf. Note this issue only arises when the leaf values are 64 bytes long before being hashed.

Since both input.blobRoot and input.bridgeRoot are user-controlled 32-byte values, the caller can exploit this by using two internal nodes from the commitment tree as inputs, resulting in successful verification. This allows for the sucessfull validation of input.bridgeRoot and input.blobRoot which are not present in the dataRootCommitment leaves.

Impact

blobRoot and bridgeRoot not present in the data commitment leaves can be validaded correctly. Enabling the non-valid bridgeRoot to be used AvailBridge._checkBridgeLeaf().

Code Snippet

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

Tool used

Manual Review.

Recommendation

Consider hashing the leaf twice, making it impossible for its collision with internal nodes of the merkle tree.

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 { same as issue 061}