From bd986f35f10aa28299b8a9b5e4633cb437b013f7 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Mon, 24 Nov 2014 13:59:42 +0100 Subject: [PATCH] Bloom filtering: check for malformed Merkle trees. Resolves issue 593. Thanks to Pieter Wiulle. --- .../org/bitcoinj/core/PartialMerkleTree.java | 20 ++++++++++-------- ...ilteredBlockAndPartialMerkleTreeTests.java | 21 +++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/PartialMerkleTree.java b/core/src/main/java/org/bitcoinj/core/PartialMerkleTree.java index 511f2437..72765d36 100644 --- a/core/src/main/java/org/bitcoinj/core/PartialMerkleTree.java +++ b/core/src/main/java/org/bitcoinj/core/PartialMerkleTree.java @@ -185,14 +185,14 @@ public class PartialMerkleTree extends Message { private Sha256Hash recursiveExtractHashes(int height, int pos, ValuesUsed used, List matchedHashes) throws VerificationException { if (used.bitsUsed >= matchedChildBits.length*8) { // overflowed the bits array - failure - throw new VerificationException("CPartialMerkleTree overflowed its bits array"); + throw new VerificationException("PartialMerkleTree overflowed its bits array"); } boolean parentOfMatch = checkBitLE(matchedChildBits, used.bitsUsed++); if (height == 0 || !parentOfMatch) { // if at height 0, or nothing interesting below, use stored hash and do not descend if (used.hashesUsed >= hashes.size()) { // overflowed the hash array - failure - throw new VerificationException("CPartialMerkleTree overflowed its hash array"); + throw new VerificationException("PartialMerkleTree overflowed its hash array"); } Sha256Hash hash = hashes.get(used.hashesUsed++); if (height == 0 && parentOfMatch) // in case of height 0, we have a matched txid @@ -201,10 +201,13 @@ public class PartialMerkleTree extends Message { } else { // otherwise, descend into the subtrees to extract matched txids and hashes byte[] left = recursiveExtractHashes(height - 1, pos * 2, used, matchedHashes).getBytes(), right; - if (pos * 2 + 1 < getTreeWidth(transactionCount, height-1)) + if (pos * 2 + 1 < getTreeWidth(transactionCount, height-1)) { right = recursiveExtractHashes(height - 1, pos * 2 + 1, used, matchedHashes).getBytes(); - else + if (Arrays.equals(right, left)) + throw new VerificationException("Invalid merkle tree with duplicated left/right branches"); + } else { right = left; + } // and combine them before returning return combineLeftRight(left, right); } @@ -223,13 +226,12 @@ public class PartialMerkleTree extends Message { * The returned root should be checked against the * merkle root contained in the block header for security. * - * @param matchedHashes A list which will contain the matched txn (will be cleared) - * Required to be a LinkedHashSet in order to retain order or transactions in the block + * @param matchedHashesOut A list which will contain the matched txn (will be cleared). * @return the merkle root of this merkle tree * @throws ProtocolException if this partial merkle tree is invalid */ - public Sha256Hash getTxnHashAndMerkleRoot(List matchedHashes) throws VerificationException { - matchedHashes.clear(); + public Sha256Hash getTxnHashAndMerkleRoot(List matchedHashesOut) throws VerificationException { + matchedHashesOut.clear(); // An empty set will not work if (transactionCount == 0) @@ -249,7 +251,7 @@ public class PartialMerkleTree extends Message { height++; // traverse the partial tree ValuesUsed used = new ValuesUsed(); - Sha256Hash merkleRoot = recursiveExtractHashes(height, 0, used, matchedHashes); + Sha256Hash merkleRoot = recursiveExtractHashes(height, 0, used, matchedHashesOut); // verify that all bits were consumed (except for the padding caused by serializing it as a byte sequence) if ((used.bitsUsed+7)/8 != matchedChildBits.length || // verify that all hashes were consumed diff --git a/core/src/test/java/org/bitcoinj/core/FilteredBlockAndPartialMerkleTreeTests.java b/core/src/test/java/org/bitcoinj/core/FilteredBlockAndPartialMerkleTreeTests.java index 2ed7534c..ede8c1bb 100644 --- a/core/src/test/java/org/bitcoinj/core/FilteredBlockAndPartialMerkleTreeTests.java +++ b/core/src/test/java/org/bitcoinj/core/FilteredBlockAndPartialMerkleTreeTests.java @@ -17,6 +17,7 @@ package org.bitcoinj.core; +import com.google.common.collect.Lists; import org.bitcoinj.core.TransactionConfidence.ConfidenceType; import org.bitcoinj.params.UnitTestParams; import org.bitcoinj.store.MemoryBlockStore; @@ -88,6 +89,26 @@ public class FilteredBlockAndPartialMerkleTreeTests extends TestWithPeerGroup { assertTrue(txns.contains(tx2.getHash())); } + private Sha256Hash numAsHash(int num) { + byte[] bits = new byte[32]; + bits[0] = (byte) num; + return new Sha256Hash(bits); + } + + @Test(expected = VerificationException.class) + public void merkleTreeMalleability() throws Exception { + List hashes = Lists.newArrayList(); + for (byte i = 1; i <= 10; i++) hashes.add(numAsHash(i)); + hashes.add(numAsHash(9)); + hashes.add(numAsHash(10)); + byte[] includeBits = new byte[2]; + Utils.setBitLE(includeBits, 9); + Utils.setBitLE(includeBits, 10); + PartialMerkleTree pmt = PartialMerkleTree.buildFromLeaves(params, includeBits, hashes); + List matchedHashes = Lists.newArrayList(); + pmt.getTxnHashAndMerkleRoot(matchedHashes); + } + @Test public void serializeDownloadBlockWithWallet() throws Exception { unitTestParams = UnitTestParams.get();