Merge #13452: rpc: have verifytxoutproof check the number of txns in proof structure

d280617bf569f84457eaea546541dc74c67cd1e4 [qa] Add a test for merkle proof malleation (Suhas Daftuar)
ed82f1700006830b6fe34572b66245c1487ccd29 have verifytxoutproof check the number of txns in proof structure (Gregory Sanders)

Pull request description:

  Recent publication of a weakness in Bitcoin's merkle tree construction demonstrates many SPV applications vulnerable to an expensive to pull off yet still plausible attack: https://bitslog.wordpress.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/

  This change would at least allow `verifytxoutproof` to properly validate that the proof matches a known block, with known number of transactions any time after the full block is processed. This should neuter the attack entirely.

  The negative is that a header-only processed block/future syncing mode would cause this to fail until the node has imported the data required.

  related: #13451

  `importprunedfunds` needs this check as well. Can expand it to cover this if people like the idea.

Tree-SHA512: 0682ec2b622a38b29f3f635323e0a8b6fc071e8a6fd134c954579926ee7b516e642966bafa667016744ce49c16e19b24dbc8801f982a36ad0a6a4aff6d93f82b

2 files changed, 15 insertions, 4 deletions

diff --git a/src/merkleblock.h b/src/merkleblock.h
index 0976e21c3a..984e33a961 100644
--- a/src/merkleblock.h
+++ b/src/merkleblock.h
@@ -115,6 +115,12 @@ public:
      * returns the merkle root, or 0 in case of failure
      */
     uint256 ExtractMatches(std::vector<uint256> &vMatch, std::vector<unsigned int> &vnIndex);
+
+    /** Get number of transactions the merkle proof is indicating for cross-reference with
+     * local blockchain knowledge.
+     */
+    unsigned int GetNumTransactions() const { return nTransactions; };
+
 };
 
 
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 63548bff05..3e06b05aca 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -306,7 +306,7 @@ static UniValue verifytxoutproof(const JSONRPCRequest& request)
             "\nArguments:\n"
             "1. \"proof\"    (string, required) The hex-encoded proof generated by gettxoutproof\n"
             "\nResult:\n"
-            "[\"txid\"]      (array, strings) The txid(s) which the proof commits to, or empty array if the proof is invalid\n"
+            "[\"txid\"]      (array, strings) The txid(s) which the proof commits to, or empty array if the proof can not be validated.\n"
         );
 
     CDataStream ssMB(ParseHexV(request.params[0], "proof"), SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS);
@@ -323,12 +323,17 @@ static UniValue verifytxoutproof(const JSONRPCRequest& request)
     LOCK(cs_main);
 
     const CBlockIndex* pindex = LookupBlockIndex(merkleBlock.header.GetHash());
-    if (!pindex || !chainActive.Contains(pindex)) {
+    if (!pindex || !chainActive.Contains(pindex) || pindex->nTx == 0) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain");
     }
 
-    for (const uint256& hash : vMatch)
-        res.push_back(hash.GetHex());
+    // Check if proof is valid, only add results if so
+    if (pindex->nTx == merkleBlock.txn.GetNumTransactions()) {
+        for (const uint256& hash : vMatch) {
+            res.push_back(hash.GetHex());
+        }
+    }
+
     return res;
 }