Bloom filtering: check for malformed Merkle trees. Resolves issue 593. Thanks to Pieter Wiulle.

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<Sha256Hash> 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)
+     * @param matchedHashesOut 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
      * @return the merkle root of this merkle tree
      * @throws ProtocolException if this partial merkle tree is invalid
      */
-    public Sha256Hash getTxnHashAndMerkleRoot(List<Sha256Hash> matchedHashes) throws VerificationException {
+    public Sha256Hash getTxnHashAndMerkleRoot(List<Sha256Hash> matchedHashesOut) throws VerificationException {
-        matchedHashes.clear();
+        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

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<Sha256Hash> 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<Sha256Hash> matchedHashes = Lists.newArrayList();
+        pmt.getTxnHashAndMerkleRoot(matchedHashes);
+    }
+
     @Test
     public void serializeDownloadBlockWithWallet() throws Exception {
         unitTestParams = UnitTestParams.get();