Permanent CVE-2017-12842 mitigation

Author: ComputerCraftr

src/consensus/tx_check.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/tx_check.h>
+#include <xep/consensus/tx_check.h>
 
 #include <consensus/amount.h>
 #include <primitives/transaction.h>
@@ -16,9 +17,13 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state)
     if (tx.vout.empty())
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-vout-empty");
     // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability)
-    if (::GetSerializeSize(TX_NO_WITNESS(tx)) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) {
+    const size_t size = ::GetSerializeSize(TX_NO_WITNESS(tx));
+    if (size * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-oversize");
     }
+    if (!XEP_CheckTransactionSize(state, size)) {
+        return false;
+    }
 
     // Check for negative or overflow output values (see CVE-2010-5139)
     CAmount nValueOut = 0;

src/policy/policy.h

@@ -32,8 +32,6 @@ 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};
 /** 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/test/data/tx_invalid.json

@@ -149,7 +149,7 @@
 [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "-1 CHECKLOCKTIMEVERIFY"]],
 "01000000010001000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000065cd1d", "CHECKLOCKTIMEVERIFY"],
 [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "1"]],
-"010000000100010000000000000000000000000000000000000000000000000000000000000000000004005194b1010000000100000000000000000002000000", "CHECKLOCKTIMEVERIFY"],
+"01000000010001000000000000000000000000000000000000000000000000000000000000000000000500005194b1010000000100000000000000000002000000", "CHECKLOCKTIMEVERIFY"],
 
 ["Input locked"],
 [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "0 CHECKLOCKTIMEVERIFY 1"]],

src/validation.cpp

@@ -794,10 +794,6 @@ 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");
-
     // 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
     // be mined yet.

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 = 0;
     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)};
     }

src/xep/consensus/tx_check.h

@@ -0,0 +1,20 @@
+// Copyright (c) 2020-2025 The XEP Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef ELECTRAPROTOCOL_CONSENSUS_TX_CHECK_H
+#define ELECTRAPROTOCOL_CONSENSUS_TX_CHECK_H
+
+#include <consensus/validation.h>
+
+bool XEP_CheckTransactionSize(TxValidationState& state, size_t size)
+{
+    // 64-byte transactions are rejected to mitigate CVE-2017-12842
+    if (size == 64) {
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "tx-size-small");
+    } else {
+        return true;
+    }
+};
+
+#endif // ELECTRAPROTOCOL_CONSENSUS_TX_CHECK_H

test/functional/data/invalid_txs.py

@@ -117,8 +117,8 @@ def get_tx(self):
 # tree depth commitment (CVE-2017-12842)
 class SizeTooSmall(BadTxTemplate):
     reject_reason = "tx-size-small"
-    expect_disconnect = False
-    valid_in_block = True
+    expect_disconnect = True
+    valid_in_block = False
 
     def get_tx(self):
         tx = CTransaction()

test/functional/feature_block.py

@@ -160,7 +160,7 @@ def run_test(self):
             blockname = f"for_invalid.{TxTemplate.__name__}"
             self.next_block(blockname)
             badtx = template.get_tx()
-            if TxTemplate != invalid_txs.InputMissing:
+            if TxTemplate != invalid_txs.InputMissing and TxTemplate != invalid_txs.SizeTooSmall:
                 self.sign_tx(badtx, attempt_spend_tx)
             badtx.rehash()
             badblock = self.update_block(blockname, [badtx])

test/functional/p2p_ibd_stalling.py

@@ -82,7 +82,7 @@ def run_test(self):
 
         # Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
         # returning the number of downloaded (but not connected) blocks.
-        bytes_recv = 172761 if not self.options.v2transport else 169692
+        bytes_recv = 174555 if not self.options.v2transport else 171486
         self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
 
         self.all_sync_send_with_ping(peers)

test/functional/p2p_invalid_tx.py

@@ -165,7 +165,7 @@ def run_test(self):
             node.p2ps[0].send_txs_and_test([rejected_parent], node, success=False)
 
         self.log.info('Test that a peer disconnection causes erase its transactions from the orphan pool')
-        with node.assert_debug_log(['Erased 100 orphan transaction(s) from peer=26']):
+        with node.assert_debug_log(['Erased 100 orphan transaction(s) from peer=']):
             self.reconnect_p2p(num_connections=1)
 
         self.log.info('Test that a transaction in the orphan pool is included in a new tip block causes erase this transaction from the orphan pool')