Merge 72ddf4237b into c5e44a0435

src/policy/policy.h

@@ -32,8 +32,8 @@ static constexpr unsigned int MINIMUM_BLOCK_RESERVED_WEIGHT{2000};
 static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000};
 /** The maximum weight for transactions we're willing to relay/mine */
 static constexpr int32_t MAX_STANDARD_TX_WEIGHT{400000};
-/** The minimum non-witness size for transactions we're willing to relay/mine: one larger than 64  */
-static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{65};
+/** In addition to transactions that are too big, do not relay/mine ones that are 64 non-witness bytes exactly: CVE-2017-12842 */
+static constexpr unsigned int NONSTANDARD_TX_NONWITNESS_SIZE{64};
 /** Maximum number of signature check operations in an IsStandard() P2SH script */
 static constexpr unsigned int MAX_P2SH_SIGOPS{15};
 /** The maximum number of sigops we're willing to relay/mine in a single tx */

src/validation.cpp

@@ -794,9 +794,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason);
     }
 
-    // Transactions smaller than 65 non-witness bytes are not relayed to mitigate CVE-2017-12842.
-    if (::GetSerializeSize(TX_NO_WITNESS(tx)) < MIN_STANDARD_TX_NONWITNESS_SIZE)
-        return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-size-small");
+    // Transactions exactly 64 non-witness bytes are not relayed to mitigate CVE-2017-12842.
+    if (::GetSerializeSize(TX_NO_WITNESS(tx)) == NONSTANDARD_TX_NONWITNESS_SIZE) {
+        return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-bad-nonwit-size");
+    }
 
     // Only accept nLockTime-using transactions that can be mined in the next
     // block; we don't want our mempool filled up with transactions that can't

src/wallet/spend.cpp

@@ -1027,7 +1027,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
     coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs;
     coin_selection_params.m_max_tx_weight = coin_control.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT);
-    int minimum_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR;
+    int minimum_tx_weight = (NONSTANDARD_TX_NONWITNESS_SIZE + 1) * WITNESS_SCALE_FACTOR;
     if (coin_selection_params.m_max_tx_weight.value() < minimum_tx_weight || coin_selection_params.m_max_tx_weight.value() > MAX_STANDARD_TX_WEIGHT) {
         return util::Error{strprintf(_("Maximum transaction weight must be between %d and %d"), minimum_tx_weight, MAX_STANDARD_TX_WEIGHT)};
     }

test/functional/data/invalid_txs.py

@@ -46,15 +46,14 @@ from test_framework.script import (
     OP_MOD,
     OP_MUL,
     OP_OR,
-    OP_RETURN,
     OP_RIGHT,
     OP_RSHIFT,
     OP_SUBSTR,
     OP_XOR,
 )
 from test_framework.script_util import (
-    MIN_PADDING,
-    MIN_STANDARD_TX_NONWITNESS_SIZE,
+    NONSTANDARD_OP_RETURN_SCRIPT,
+    NONSTANDARD_TX_NONWITNESS_SIZE,
     script_to_p2sh_script,
 )
 basic_p2sh = script_to_p2sh_script(CScript([OP_0]))
@@ -115,17 +114,16 @@ class InputMissing(BadTxTemplate):
 
 # The following check prevents exploit of lack of merkle
 # tree depth commitment (CVE-2017-12842)
-class SizeTooSmall(BadTxTemplate):
-    reject_reason = "tx-size-small"
+class BadNonWitSize(BadTxTemplate):
+    reject_reason = "tx-bad-nonwit-size"
     expect_disconnect = False
     valid_in_block = True
 
     def get_tx(self):
         tx = CTransaction()
         tx.vin.append(self.valid_txin)
-        tx.vout.append(CTxOut(0, CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 2)))))
-        assert len(tx.serialize_without_witness()) == 64
-        assert MIN_STANDARD_TX_NONWITNESS_SIZE - 1 == 64
+        tx.vout.append(CTxOut(0, NONSTANDARD_OP_RETURN_SCRIPT))
+        assert len(tx.serialize_without_witness()) == NONSTANDARD_TX_NONWITNESS_SIZE
         tx.calc_sha256()
         return tx
 

test/functional/mempool_accept.py

@@ -33,17 +33,16 @@ from test_framework.script import (
     sign_input_legacy,
 )
 from test_framework.script_util import (
-    DUMMY_MIN_OP_RETURN_SCRIPT,
+    INVALID_SPK_LEN,
     keys_to_multisig_script,
-    MIN_PADDING,
-    MIN_STANDARD_TX_NONWITNESS_SIZE,
+    NONSTANDARD_OP_RETURN_SCRIPT,
+    NONSTANDARD_TX_NONWITNESS_SIZE,
     PAY_TO_ANCHOR,
     script_to_p2sh_script,
     script_to_p2wsh_script,
 )
 from test_framework.util import (
     assert_equal,
-    assert_greater_than,
     assert_raises_rpc_error,
     sync_txindex,
 )
@@ -352,30 +351,35 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
             maxfeerate=0,
         )
 
-        # Prep for tiny-tx tests with wsh(OP_TRUE) output
-        seed_tx = self.wallet.send_to(from_node=node, scriptPubKey=script_to_p2wsh_script(CScript([OP_TRUE])), amount=COIN)
+        # Prep for tiny-tx tests with MiniWallet anyone-can-spend output
+        seed_utxo = self.wallet.send_self_transfer(from_node=node)["new_utxo"]
         self.generate(node, 1)
 
         self.log.info('A tiny transaction(in non-witness bytes) that is disallowed')
-        tx = CTransaction()
-        tx.vin.append(CTxIn(COutPoint(int(seed_tx["txid"], 16), seed_tx["sent_vout"]), b"", SEQUENCE_FINAL))
-        tx.wit.vtxinwit = [CTxInWitness()]
-        tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
-        tx.vout.append(CTxOut(0, CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 2)))))
+        tx = self.wallet.create_self_transfer(utxo_to_spend=seed_utxo)["tx"]
+        tx.vout[0].scriptPubKey = NONSTANDARD_OP_RETURN_SCRIPT
         # Note it's only non-witness size that matters!
-        assert_equal(len(tx.serialize_without_witness()), 64)
-        assert_equal(MIN_STANDARD_TX_NONWITNESS_SIZE - 1, 64)
-        assert_greater_than(len(tx.serialize()), 64)
+        assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE)
+        assert len(tx.serialize()) != NONSTANDARD_TX_NONWITNESS_SIZE
 
         self.check_mempool_result(
-            result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-size-small'}],
+            result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-bad-nonwit-size'}],
             rawtxs=[tx.serialize().hex()],
             maxfeerate=0,
         )
 
-        self.log.info('Minimally-small transaction(in non-witness bytes) that is allowed')
-        tx.vout[0] = CTxOut(COIN - 1000, DUMMY_MIN_OP_RETURN_SCRIPT)
-        assert_equal(len(tx.serialize_without_witness()), MIN_STANDARD_TX_NONWITNESS_SIZE)
+        self.log.info('Just-below size transaction(in non-witness bytes) that is allowed')
+        tx.vout[0] = CTxOut(int(seed_utxo["value"] * COIN) - 1000, CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN - 2))))
+        assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE - 1)
+        self.check_mempool_result(
+            result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}],
+            rawtxs=[tx.serialize().hex()],
+            maxfeerate=0,
+        )
+
+        self.log.info('Just-above size transaction(in non-witness bytes) that is allowed')
+        tx.vout[0] = CTxOut(int(seed_utxo["value"] * COIN) - 1000, CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN))))
+        assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE + 1)
         self.check_mempool_result(
             result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}],
             rawtxs=[tx.serialize().hex()],

test/functional/rpc_psbt.py

@@ -34,7 +34,7 @@ from test_framework.psbt import (
     PSBT_OUT_TAP_TREE,
 )
 from test_framework.script import CScript, OP_TRUE
-from test_framework.script_util import MIN_STANDARD_TX_NONWITNESS_SIZE
+from test_framework.script_util import NONSTANDARD_TX_NONWITNESS_SIZE
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_not_equal,
@@ -213,7 +213,7 @@ class PSBTTest(BitcoinTestFramework):
 
         self.log.info("Test for invalid maximum transaction weights")
         dest_arg = [{self.nodes[0].getnewaddress(): 1}]
-        min_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR
+        min_tx_weight = (NONSTANDARD_TX_NONWITNESS_SIZE + 1) * WITNESS_SCALE_FACTOR
         assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": -1})
         assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": 0})
         assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": MAX_STANDARD_TX_WEIGHT + 1})

test/functional/test_framework/script_util.py

@@ -22,8 +22,8 @@ from test_framework.script import (
     sha256,
 )
 
-# To prevent a "tx-size-small" policy rule error, a transaction has to have a
-# non-witness size of at least 65 bytes (MIN_STANDARD_TX_NONWITNESS_SIZE in
+# To prevent a "tx-bad-nonwit-size" policy rule error, a transaction may not have a
+# non-witness size of 64 bytes (NONSTANDARD_TX_NONWITNESS_SIZE in
 # src/policy/policy.h). Considering a Tx with the smallest possible single
 # input (blank, empty scriptSig), and with an output omitting the scriptPubKey,
 # we get to a minimum size of 60 bytes:
@@ -32,16 +32,15 @@ from test_framework.script import (
 # Blank Input: 32 [PrevTxHash] + 4 [Index] + 1 [scriptSigLen] + 4 [SeqNo] = 41 bytes
 # Output:      8 [Amount] + 1 [scriptPubKeyLen] = 9 bytes
 #
-# Hence, the scriptPubKey of the single output has to have a size of at
-# least 5 bytes.
-MIN_STANDARD_TX_NONWITNESS_SIZE = 65
-MIN_PADDING = MIN_STANDARD_TX_NONWITNESS_SIZE - 10 - 41 - 9
-assert MIN_PADDING == 5
+# Hence, the scriptPubKey of the single output MUST NOT have a size of 4.
+NONSTANDARD_TX_NONWITNESS_SIZE = 64
+INVALID_SPK_LEN = NONSTANDARD_TX_NONWITNESS_SIZE - 10 - 41 - 9
+assert INVALID_SPK_LEN == 4
 
 # This script cannot be spent, allowing dust output values under
 # standardness checks
-DUMMY_MIN_OP_RETURN_SCRIPT = CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 1)))
-assert len(DUMMY_MIN_OP_RETURN_SCRIPT) == MIN_PADDING
+NONSTANDARD_OP_RETURN_SCRIPT = CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN - 1)))
+assert len(NONSTANDARD_OP_RETURN_SCRIPT) == INVALID_SPK_LEN
 
 PAY_TO_ANCHOR = CScript([OP_1, bytes.fromhex("4e73")])