diff --git a/core/packages/contracts/scripts/helpers.ts b/core/packages/contracts/scripts/helpers.ts index b15c599f2a..69355595b5 100644 --- a/core/packages/contracts/scripts/helpers.ts +++ b/core/packages/contracts/scripts/helpers.ts @@ -67,7 +67,7 @@ class ValidatorSet { let leaves = wallets.map((w) => keccakFromHexString(w.address)) let tree = new MerkleTree(leaves, keccak, { sortLeaves: false, - sortPairs: true, + sortPairs: false, }) this.wallets = wallets diff --git a/core/packages/contracts/src/BeefyClient.sol b/core/packages/contracts/src/BeefyClient.sol index 03a981a664..1f89415c8e 100644 --- a/core/packages/contracts/src/BeefyClient.sol +++ b/core/packages/contracts/src/BeefyClient.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.19; import {ECDSA} from "openzeppelin/utils/cryptography/ECDSA.sol"; import {Ownable} from "openzeppelin/access/Ownable.sol"; -import {MerkleProof} from "openzeppelin/utils/cryptography/MerkleProof.sol"; +import {MerkleProof} from "./utils/MerkleProof.sol"; import {Bitfield} from "./utils/Bitfield.sol"; import {MMRProof} from "./utils/MMRProof.sol"; import {ScaleCodec} from "./ScaleCodec.sol"; @@ -441,10 +441,7 @@ contract BeefyClient is Ownable { } // Check that validator is actually in a validator set - // - // NOTE: This currently insecure due to a regression documented in SNO-427. - // Basically `proof.index` (the merkle leaf index) is not being validated. - if (!isValidatorInSet(vset, proof.account, proof.proof)) { + if (!isValidatorInSet(vset, proof.account, proof.index, proof.proof)) { revert InvalidValidatorProof(); } @@ -498,13 +495,13 @@ contract BeefyClient is Ownable { * @param proof Merkle proof required for validation of the address * @return true if the validator is in the set */ - function isValidatorInSet(ValidatorSet memory vset, address addr, bytes32[] calldata proof) + function isValidatorInSet(ValidatorSet memory vset, address addr, uint256 index, bytes32[] calldata proof) internal pure returns (bool) { bytes32 hashedLeaf = keccak256(abi.encodePacked(addr)); - return MerkleProof.verify(proof, vset.root, hashedLeaf); + return MerkleProof.verifyMerkleLeafAtPosition(vset.root, hashedLeaf, index, vset.length, proof); } /** diff --git a/core/packages/contracts/src/ParachainClient.sol b/core/packages/contracts/src/ParachainClient.sol index 3c10775a34..af4e976901 100644 --- a/core/packages/contracts/src/ParachainClient.sol +++ b/core/packages/contracts/src/ParachainClient.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.19; -import "openzeppelin/utils/cryptography/MerkleProof.sol"; +import "./utils/MerkleProof.sol"; import "./BeefyClient.sol"; import "./IParachainClient.sol"; import "./ScaleCodec.sol"; @@ -74,7 +74,12 @@ contract ParachainClient is IParachainClient { bytes32 parachainHeadHash = createParachainHeaderMerkleLeaf(proof.header); // Compute the merkle root hash of all parachain heads - bytes32 parachainHeadsRoot = MerkleProof.processProof(proof.headProof.proof, parachainHeadHash); + bytes32 parachainHeadsRoot = MerkleProof.computeRootFromProofAtPosition( + parachainHeadHash, + proof.headProof.pos, + proof.headProof.width, + proof.headProof.proof + ); bytes32 leafHash = createMMRLeaf(proof.leafPartial, parachainHeadsRoot); return beefyClient.verifyMMRLeafProof(leafHash, proof.leafProof, proof.leafProofOrder); diff --git a/core/packages/contracts/src/utils/Bitfield.sol b/core/packages/contracts/src/utils/Bitfield.sol index 06e62d185a..221304b871 100644 --- a/core/packages/contracts/src/utils/Bitfield.sol +++ b/core/packages/contracts/src/utils/Bitfield.sol @@ -82,7 +82,7 @@ library Bitfield { * The algorithm below is implemented after https://en.wikipedia.org/wiki/Hamming_weight#Efficient_implementation. * Further improvements are possible, see the article above. */ - function countSetBits(uint256[] memory self) public pure returns (uint256) { + function countSetBits(uint256[] memory self) internal pure returns (uint256) { unchecked { uint256 count = 0; for (uint256 i = 0; i < self.length; i++) { diff --git a/core/packages/contracts/src/utils/MerkleProof.sol b/core/packages/contracts/src/utils/MerkleProof.sol new file mode 100644 index 0000000000..2cfa8bba9b --- /dev/null +++ b/core/packages/contracts/src/utils/MerkleProof.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.19; + +library MerkleProof { + /** + * @notice Verify that a specific leaf element is part of the Merkle Tree at a specific position in the tree + * + * @param root the root of the merkle tree + * @param leaf the leaf which needs to be proven + * @param pos the position of the leaf, index starting with 0 + * @param width the width or number of leaves in the tree + * @param proof the array of proofs to help verify the leaf's membership, ordered from leaf to root + * @return a boolean value representing the success or failure of the operation + */ + function verifyMerkleLeafAtPosition( + bytes32 root, + bytes32 leaf, + uint256 pos, + uint256 width, + bytes32[] memory proof + ) internal pure returns (bool) { + bytes32 computedHash = computeRootFromProofAtPosition(leaf, pos, width, proof); + + return computedHash == root; + } + + function computeRootFromProofAtPosition( + bytes32 leaf, + uint256 pos, + uint256 width, + bytes32[] memory proof + ) internal pure returns (bytes32) { + bytes32 computedHash = leaf; + + require(pos < width, "Merkle position is too high"); + + unchecked { + uint256 i = 0; + for (uint256 height = 0; width > 1; height++) { + bool computedHashLeft = pos & 1 == 0; + + // check if at rightmost branch and whether the computedHash is left + if (pos + 1 == width && computedHashLeft) { + // there is no sibling and also no element in proofs, so we just go up one layer in the tree + pos = pos >> 1; + width = ((width - 1) >> 1) + 1; + continue; + } + + require(i < proof.length, "Merkle proof is too short"); + + if (computedHashLeft) { + computedHash = efficientHash(computedHash, proof[i]); + } else { + computedHash = efficientHash(proof[i], computedHash); + } + + pos = pos >> 1; + width = ((width - 1) >> 1) + 1; + i++; + } + + return computedHash; + } + } + + function efficientHash(bytes32 a, bytes32 b) private pure returns (bytes32 value) { + /// @solidity memory-safe-assembly + assembly { + mstore(0x00, a) + mstore(0x20, b) + value := keccak256(0x00, 0x40) + } + } +} diff --git a/core/packages/test/scripts/set-env.sh b/core/packages/test/scripts/set-env.sh index 968ad5c08d..30c6dcfe4c 100755 --- a/core/packages/test/scripts/set-env.sh +++ b/core/packages/test/scripts/set-env.sh @@ -1,6 +1,6 @@ root_dir="$(realpath ../../..)" bridge_hub_runtime="${PARACHAIN_RUNTIME:-bridge-hub-rococo-local}" -relaychain_version="${POLKADOT_VER:-v0.9.42}" +relaychain_version="${POLKADOT_VER:-v0.9.43}" relaychain_dir="$root_dir/parachain/.cargo/$relaychain_version" relaychain_bin="${POLKADOT_BIN:-$relaychain_dir/bin/polkadot}" cumulus_version="${CUMULUS_VER:-snowbridge}"