From 59391d5399eb96164dce3af7958649fe2d3dccca Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 20:48:20 -0500 Subject: [PATCH] Merge bitcoin/bitcoin#35984: sign: skip signing SIGHASH_SINGLE inputs with no corresponding output 8df006f233839736b6d7b856bcf8673526c7acef 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 #35977 ACKs for top commit: thomasbuilds: ACK 8df006f achow101: ACK 8df006f233839736b6d7b856bcf8673526c7acef 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 Co-Authored-By: Claude Opus 5.5 --- src/script/sign.cpp | 12 ++++++------ test/functional/rpc_psbt.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 8fdb7a192a2d..8df3235a24d8 100644 --- a/src/script/sign.cpp +++ b/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& coins, int nHashType, std::map& 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); diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 13ddfbbe91e9..feb369b7fa83 100755 --- a/test/functional/rpc_psbt.py +++ b/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()