Merge bitcoin#35984: sign: skip signing SIGHASH_SINGLE inputs with no corresponding output

8df006f wallet: skip signing SIGHASH_SINGLE inputs with no corresponding output (furszy)

Pull request description:

  `SIGHASH_SINGLE` only commits to the output at the input's index. If the output at such
  position doesn't exist, it commits to no output at all (legacy uses a fixed sighash of 1, segwit
  v0 zeroes `hashOutputs`), which means the signature stays valid even when outputs are
  swapped, which is a footgun that lets funds be redirected without the owner's consent.

  `SignTransaction()` already skipped these inputs, but `SignPSBTInput()` did not, so
  `walletprocesspsbt` signed them. This moves the check into the `CreateSig` so both
  paths, and any future one, skip producing the detached signature.

  Note: if there is a valid use for the segwit v0 case, I would rather re-allow it through
  an explicit opt-in arg than by default, so it is always a deliberate choice.

  Fixes bitcoin#35977

ACKs for top commit:
  thomasbuilds:
    ACK 8df006f
  achow101:
    ACK 8df006f

Tree-SHA512: ac1c9659910eae889475c65b8dde68e711c4d85d0d520af06411728591e573bf027b41a9ac6dd0f6e93de425841ac6c7d9e6c9f07936b9d8ae390941fbe6f1f1

BACKPORT NOTE:
- src/script/sign.cpp: Dash has no SignOptions or SIGHASH_DEFAULT, so the
  check uses nHashType. Dash also has no SIGHASH_OUTPUT_MASK (added with
  Taproot), so it tests (nHashType & 0x1f), the same test Dash's legacy
  SignatureHash uses for SIGHASH_SINGLE. For every sighash the wallet can
  sign with (ALL/NONE/SINGLE, optionally |ANYONECANPAY) this matches
  upstream's (hashtype & SIGHASH_OUTPUT_MASK).
- test/functional/rpc_psbt.py, test_sighash_single():
  - No loop over legacy/bech32/bech32m address types; Dash only has P2PKH.
    Instead the test covers SINGLE, SINGLE|ANYONECANPAY and ALL (control),
    and also checks signrawtransactionwithwallet, because this change
    removes SignTransaction's own SIGHASH_SINGLE check and Dash had no test
    for that path.
  - It runs after test_input_confs_control; Dash has none of the sighash
    tests upstream places it next to.
  - Omitted: the test_psbt_roundtrip and test_psbt_version UTXO selection
    changes; those tests (PSBTv2) do not exist in Dash.

Co-authored-by: Ava Chow <github@achow101.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: 3 people

src/script/sign.cpp

@@ -37,6 +37,11 @@ bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provid
     if (!provider.GetKey(address, key))
         return false;
 
+    // If an input is signed with SIGHASH_SINGLE but there is no output at the same index, the
+    // signature commits to no output at all. Which means such a signature stays valid if the
+    // output is swapped, which is a footgun. So don't produce it.
+    if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn >= m_txto.vout.size()) return false;
+
     uint256 hash = SignatureHash(scriptCode, m_txto, nIn, nHashType, amount, sigversion, m_txdata);
     if (!key.Sign(hash, vchSig))
         return false;
@@ -400,8 +405,6 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script)
 
 bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, int nHashType, std::map<int, bilingual_str>& input_errors)
 {
-    bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE);
-
     // Use CTransaction for the constant parts of the
     // transaction to avoid rehashing.
     const CTransaction txConst(mtx);
@@ -437,10 +440,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore,
         const CAmount& amount = coin->second.out.nValue;
 
         SignatureData sigdata = DataFromTransaction(mtx, i, coin->second.out);
-        // Only sign SIGHASH_SINGLE if there's a corresponding output:
-        if (!fHashSingle || (i < mtx.vout.size())) {
-            ProduceSignature(*keystore, MutableTransactionSignatureCreator(mtx, i, amount, &txdata, nHashType), prevPubKey, sigdata);
-        }
+        ProduceSignature(*keystore, MutableTransactionSignatureCreator(mtx, i, amount, &txdata, nHashType), prevPubKey, sigdata);
 
         UpdateInput(txin, sigdata);
 

test/functional/rpc_psbt.py

@@ -108,6 +108,38 @@ def test_input_confs_control(self):
 
         wallet.unloadwallet()
 
+    def test_sighash_single(self):
+        self.log.info("Test that SIGHASH_SINGLE won't sign an input with no matching output")
+        node = self.nodes[0]
+        node.createwallet("sighash_single")
+        wallet = node.get_wallet_rpc("sighash_single")
+        def_wallet = node.get_wallet_rpc(self.default_wallet_name)
+
+        addrs = [wallet.getnewaddress() for _ in range(2)]
+        for addr in addrs:
+            def_wallet.sendtoaddress(addr, 1)
+        self.generatetoaddress(node, 1, def_wallet.getnewaddress())
+        node.syncwithvalidationinterfacequeue()
+        ins = [{"txid": u["txid"], "vout": u["vout"]} for u in wallet.listunspent(addresses=addrs)]
+        assert_equal(len(ins), 2)
+
+        raw = node.createrawtransaction(ins, [{wallet.getnewaddress(): 1.9999}])
+        for sighash in ["SINGLE", "SINGLE|ANYONECANPAY", "ALL"]:
+            signed = wallet.walletprocesspsbt(node.converttopsbt(raw), True, sighash)["psbt"]
+            state = wallet.analyzepsbt(signed)["inputs"]
+            # Input 0 has a matching output and can always sign and finalize.
+            assert state[0]["is_final"]
+            # Input 1 has no matching output, so only ALL can sign it.
+            assert_equal(state[1]["is_final"], sighash == "ALL")
+
+            signed_raw = wallet.signrawtransactionwithwallet(raw, [], sighash)
+            assert_equal(signed_raw["complete"], sighash == "ALL")
+            tx = node.decoderawtransaction(signed_raw["hex"])
+            assert tx["vin"][0]["scriptSig"]["hex"]
+            assert_equal(bool(tx["vin"][1]["scriptSig"]["hex"]), sighash == "ALL")
+
+        wallet.unloadwallet()
+
     def run_test(self):
         # Create and fund a raw tx for sending 10 DASH
         psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt']
@@ -441,6 +473,7 @@ def run_test(self):
             assert_equal(extracted, extractor['result'])
 
         self.test_input_confs_control()
+        self.test_sighash_single()
 
         # Test that psbts with p2pkh outputs are created properly
         p2pkh = self.nodes[0].getnewaddress()