From 906b6d9da6a6b2e6a5f1d9046b3b9c2c7e490c99 Mon Sep 17 00:00:00 2001
From: Antoine Riard <dev@ariard.me>
Date: Thu, 13 May 2021 12:21:23 -0400
Subject: [PATCH 1/2] test: Extend feature_rbf.py with no inherited signaling

---
 test/functional/feature_rbf.py | 67 ++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py
index 945880cc3b8..f0f1c1905ce 100755
--- a/test/functional/feature_rbf.py
+++ b/test/functional/feature_rbf.py
@@ -115,6 +115,9 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         self.log.info("Running test prioritised transactions...")
         self.test_prioritised_transactions()
 
+        self.log.info("Running test no inherited signaling...")
+        self.test_no_inherited_signaling()
+
         self.log.info("Passed")
 
     def test_simple_doublespend(self):
@@ -563,5 +566,69 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         assert_equal(json0["vin"][0]["sequence"], 4294967293)
         assert_equal(json1["vin"][0]["sequence"], 4294967294)
 
+    def test_no_inherited_signaling(self):
+        # Send tx from which to conflict outputs later
+        base_txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10"))
+        self.nodes[0].generate(1)
+        self.sync_blocks()
+
+        # Create an explicitly opt-in parent transaction
+        optin_parent_tx = self.nodes[0].createrawtransaction([{
+            'txid': base_txid,
+            'vout': 0,
+            "sequence": 0xfffffffd,
+        }], {self.nodes[0].getnewaddress(): Decimal("9.99998")})
+
+        optin_parent_tx = self.nodes[0].signrawtransactionwithwallet(optin_parent_tx)
+
+        # Broadcast parent tx
+        optin_parent_txid = self.nodes[0].sendrawtransaction(hexstring=optin_parent_tx["hex"], maxfeerate=0)
+        assert optin_parent_txid in self.nodes[0].getrawmempool()
+
+        replacement_parent_tx = self.nodes[0].createrawtransaction([{
+            'txid': base_txid,
+            'vout': 0,
+            "sequence": 0xfffffffd,
+        }], {self.nodes[0].getnewaddress(): Decimal("9.90000")})
+
+        replacement_parent_tx = self.nodes[0].signrawtransactionwithwallet(replacement_parent_tx)
+
+        # Test if parent tx can be replaced.
+        res = self.nodes[0].testmempoolaccept(rawtxs=[replacement_parent_tx['hex']], maxfeerate=0)[0]
+
+        # Parent can be replaced.
+        assert_equal(res['allowed'], True)
+
+        # Create an opt-out child tx spending the opt-in parent
+        optout_child_tx = self.nodes[0].createrawtransaction([{
+            'txid': optin_parent_txid,
+            'vout': 0,
+            "sequence": 0xffffffff,
+        }], {self.nodes[0].getnewaddress(): Decimal("9.99990")})
+
+        optout_child_tx = self.nodes[0].signrawtransactionwithwallet(optout_child_tx)
+
+        # Broadcast child tx
+        optout_child_txid = self.nodes[0].sendrawtransaction(hexstring=optout_child_tx["hex"], maxfeerate=0)
+        assert optout_child_txid in self.nodes[0].getrawmempool()
+
+        replacement_child_tx = self.nodes[0].createrawtransaction([{
+            'txid': optin_parent_txid,
+            'vout': 0,
+            "sequence": 0xffffffff,
+        }], {self.nodes[0].getnewaddress(): Decimal("9.00000")})
+
+        replacement_child_tx = self.nodes[0].signrawtransactionwithwallet(replacement_child_tx)
+
+        # Broadcast replacement child tx
+        # BIP 125 :
+        # 1. The original transactions signal replaceability explicitly or through inheritance as described in the above
+        # Summary section.
+        # The original transaction (`optout_child_tx`) doesn't signal RBF but its parent (`optin_parent_txid`) does.
+        # The replacement transaction (`replacement_child_tx`) should be able to replace the original transaction.
+        # See CVE-2021-31876 for further explanations.
+        assert optin_parent_txid in self.nodes[0].getrawmempool()
+        assert_raises_rpc_error(-26, 'txn-mempool-conflict', self.nodes[0].sendrawtransaction, replacement_child_tx["hex"], 0)
+
 if __name__ == '__main__':
     ReplaceByFeeTest().main()

From 2eb0eeda39cab997f9a5f817f7c12e7fffeaf35a Mon Sep 17 00:00:00 2001
From: Antoine Riard <dev@ariard.me>
Date: Thu, 13 May 2021 12:35:33 -0400
Subject: [PATCH 2/2] validation: document lack of inherited signaling in RBF
 policy

---
 src/validation.cpp | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 4f9b8687b77..9826616cb26 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -625,10 +625,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
                 // is for the sake of multi-party protocols, where we don't
                 // want a single party to be able to disable replacement.
                 //
-                // The opt-out ignores descendants as anyone relying on
-                // first-seen mempool behavior should be checking all
-                // unconfirmed ancestors anyway; doing otherwise is hopelessly
-                // insecure.
+                // Transactions that don't explicitly signal replaceability are
+                // *not* replaceable with the current logic, even if one of their
+                // unconfirmed ancestors signals replaceability. This diverges
+                // from BIP125's inherited signaling description (see CVE-2021-31876).
+                // Applications relying on first-seen mempool behavior should
+                // check all unconfirmed ancestors; otherwise an opt-in ancestor
+                // might be replaced, causing removal of this descendant.
                 bool fReplacementOptOut = true;
                 for (const CTxIn &_txin : ptxConflicting->vin)
                 {