Merge #14696: qa: Add explicit references to related CVE's in p2p_invalid_block test.

0c62e3aa73839e97e65a3155e06a98d84b700a1e New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. (lucash-dev)
38bfca6bb2ad68719415e9c54a981441052da072 Added comments referencing multiple CVEs in tests and production code. (lucash-dev)

Pull request description:

  This functional test includes two scenarios that test for regressions of vulnerabilities, but they are only briefly described. There are freely available documents explaining in detail the issues, but without explicit mentions, the developer trying to maintain the code needs an additional step of digging in commit history and PR conversations to figure it out.
  Added comments to explicitly mention  CVE-2018-17144 and CVE-2012-2459, for more complete documentation.
  This improves developer experience by making understanding the tests easier.

ACKs for top commit:
  laanwj:
    ACK 0c62e3aa73839e97e65a3155e06a98d84b700a1e, checked the CVE numbers, thanks for adding documentation

Tree-SHA512: 3ee05351745193b8b959e4a25d50f25a693b2d24b0732ed53cf7d5882df40b5dd0f1877bd5c69cffb921d4a7acf9deb3cc1160b96dc730d9b5984151ad06b7c9

4 files changed, 93 insertions, 10 deletions

diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 454eb583f7..9dc06422c4 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functional/data/invalid_txs.py
@@ -24,7 +24,24 @@ import abc
 from test_framework.messages import CTransaction, CTxIn, CTxOut, COutPoint
 from test_framework import script as sc
 from test_framework.blocktools import create_tx_with_script, MAX_BLOCK_SIGOPS
-
+from test_framework.script import (
+    CScript,
+    OP_CAT,
+    OP_SUBSTR,
+    OP_LEFT,
+    OP_RIGHT,
+    OP_INVERT,
+    OP_AND,
+    OP_OR,
+    OP_XOR,
+    OP_2MUL,
+    OP_2DIV,
+    OP_MUL,
+    OP_DIV,
+    OP_MOD,
+    OP_LSHIFT,
+    OP_RSHIFT
+)
 basic_p2sh = sc.CScript([sc.OP_HASH160, sc.hash160(sc.CScript([sc.OP_0])), sc.OP_EQUAL])
 
 
@@ -178,7 +195,44 @@ class TooManySigops(BadTxTemplate):
             script_pub_key=lotsa_checksigs,
             amount=1)
 
+def getDisabledOpcodeTemplate(opcode):
+    """ Creates disabled opcode tx template class"""
+    def get_tx(self):
+        tx = CTransaction()
+        vin = self.valid_txin
+        vin.scriptSig = CScript([opcode])
+        tx.vin.append(vin)
+        tx.vout.append(CTxOut(1, basic_p2sh))
+        tx.calc_sha256()
+        return tx
+
+    return type('DisabledOpcode_' + str(opcode), (BadTxTemplate,), {
+        'reject_reason': "disabled opcode",
+        'expect_disconnect': True,
+        'get_tx': get_tx,
+        'valid_in_block' : True
+        })
+
+# Disabled opcode tx templates (CVE-2010-5137)
+DisabledOpcodeTemplates = [getDisabledOpcodeTemplate(opcode) for opcode in [
+    OP_CAT,
+    OP_SUBSTR,
+    OP_LEFT,
+    OP_RIGHT,
+    OP_INVERT,
+    OP_AND,
+    OP_OR,
+    OP_XOR,
+    OP_2MUL,
+    OP_2DIV,
+    OP_MUL,
+    OP_DIV,
+    OP_MOD,
+    OP_LSHIFT,
+    OP_RSHIFT]]
+
 
 def iter_all_templates():
     """Iterate through all bad transaction template types."""
     return BadTxTemplate.__subclasses__()
+
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 377e6c82cd..c74270febc 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -806,7 +806,7 @@ class FullBlockTest(BitcoinTestFramework):
         #
         # Blocks are not allowed to contain a transaction whose id matches that of an earlier,
         # not-fully-spent transaction in the same chain. To test, make identical coinbases;
-        # the second one should be rejected.
+        # the second one should be rejected. See also CVE-2012-1909.
         #
         self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)")
         self.move_tip(60)
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index 209a222004..e19e249f21 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -211,6 +211,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
             rawtxs=[tx.serialize().hex()],
         )
 
+        # The following two validations prevent overflow of the output amounts (see CVE-2010-5139).
         self.log.info('A transaction with too large output value')
         tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
         tx.vout[0].nValue = 21000000 * COIN + 1
diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py
index 1e0b876593..905534b862 100755
--- a/test/functional/p2p_invalid_block.py
+++ b/test/functional/p2p_invalid_block.py
@@ -53,10 +53,11 @@ class InvalidBlockRequestTest(BitcoinTestFramework):
         block_time = best_block["time"] + 1
 
         # Use merkle-root malleability to generate an invalid block with
-        # same blockheader.
+        # same blockheader (CVE-2012-2459).
         # Manufacture a block with 3 transactions (coinbase, spend of prior
         # coinbase, spend of that spend).  Duplicate the 3rd transaction to
         # leave merkle root and blockheader unchanged but invalidate the block.
+        # For more information on merkle-root malleability see src/consensus/merkle.cpp.
         self.log.info("Test merkle root malleability.")
 
         block2 = create_block(tip, create_coinbase(height), block_time)
@@ -81,15 +82,16 @@ class InvalidBlockRequestTest(BitcoinTestFramework):
 
         node.p2p.send_blocks_and_test([block2], node, success=False, reject_reason='bad-txns-duplicate')
 
-        # Check transactions for duplicate inputs
+        # Check transactions for duplicate inputs (CVE-2018-17144)
         self.log.info("Test duplicate input block.")
 
-        block2_orig.vtx[2].vin.append(block2_orig.vtx[2].vin[0])
-        block2_orig.vtx[2].rehash()
-        block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root()
-        block2_orig.rehash()
-        block2_orig.solve()
-        node.p2p.send_blocks_and_test([block2_orig], node, success=False, reject_reason='bad-txns-inputs-duplicate')
+        block2_dup = copy.deepcopy(block2_orig)
+        block2_dup.vtx[2].vin.append(block2_dup.vtx[2].vin[0])
+        block2_dup.vtx[2].rehash()
+        block2_dup.hashMerkleRoot = block2_dup.calc_merkle_root()
+        block2_dup.rehash()
+        block2_dup.solve()
+        node.p2p.send_blocks_and_test([block2_dup], node, success=False, reject_reason='bad-txns-inputs-duplicate')
 
         self.log.info("Test very broken block.")
 
@@ -105,5 +107,31 @@ class InvalidBlockRequestTest(BitcoinTestFramework):
         node.p2p.send_blocks_and_test([block3], node, success=False, reject_reason='bad-cb-amount')
 
 
+        # Complete testing of CVE-2012-2459 by sending the original block.
+        # It should be accepted even though it has the same hash as the mutated one.
+
+        self.log.info("Test accepting original block after rejecting its mutated version.")
+        node.p2p.send_blocks_and_test([block2_orig], node, success=True, timeout=5)
+
+        # Update tip info
+        height += 1
+        block_time += 1
+        tip = int(block2_orig.hash, 16)
+
+        # Complete testing of CVE-2018-17144, by checking for the inflation bug.
+        # Create a block that spends the output of a tx in a previous block.
+        block4 = create_block(tip, create_coinbase(height), block_time)
+        tx3 = create_tx_with_script(tx2, 0, script_sig=b'\x51', amount=50 * COIN)
+
+        # Duplicates input
+        tx3.vin.append(tx3.vin[0])
+        tx3.rehash()
+        block4.vtx.append(tx3)
+        block4.hashMerkleRoot = block4.calc_merkle_root()
+        block4.rehash()
+        block4.solve()
+        self.log.info("Test inflation by duplicating input")
+        node.p2p.send_blocks_and_test([block4], node, success=False,  reject_reason='bad-txns-inputs-duplicate')
+
 if __name__ == '__main__':
     InvalidBlockRequestTest().main()