Merge pull request #1046 from kdmukai/2026_09_psbt_nested_singlesig_change
What changed, and why it matters
This commit fixes a security bug in how SeedSigner recognizes 'change' outputs on bitcoin transactions when using older nested SegWit single-signature wallets. A change output is change that comes back to your own wallet; the device must identify it correctly so it does not look like money being sent to a stranger. Because of an optional field in the PSBT data format, a valid change output could be mistaken for a plain pay-to-script-hash output and treated as an external spend. The fix lets the parser consider these outputs as change candidates and then verifies ownership by rebuilding the expected script from the seed's own key, rather than trusting the PSBT's missing or misleading redeem script. The commit also adds tests showing that malicious PSBTs that claim ownership but pay someone else are rejected.
Review the ownership rebuild path in _parse_outputs to confirm it is invoked for all outputs passing _is_change_candidate, and ensure that no other policy-shape comparison bypasses the carve-out. Consider adding tests for partially signed or multi-input PSBTs where only some inputs are p2sh-p2wpkh. Users should upgrade to a release containing this commit before signing nested SegWit single-sig PSBTs produced by wallets that omit the redeem script.
Security signals we found
Change-output misclassification could cause users to believe change is being sent to an external address
PSBT field omission (redeem_script) used to bypass change verification
Fix shifts from trust in PSBT metadata to cryptographic rebuild from seed-derived key
Tests include adversarial PSBTs that claim ownership but pay foreign keys
Commit title and message explicitly tag [security]
Evidence from the diff
The patch changes PSBTParser._policy_shape_matches into _is_change_candidate. Previously, change detection compared only policy shape (type, m, n), so a p2sh-p2wpkh input whose change output omitted its redeem script (allowed by BIP-174) parsed as bare p2sh and failed the shape match, causing it to be classified as spend. The new method adds a carve-out: when inputs are p2sh-p2wpkh, the output parses as p2sh, and the output’s redeem_script is None, the output is still treated as a change candidate. Full ownership verification is then performed by rebuilding p2sh(p2wpkh(K)) from the seed-derived key at the claimed derivation path and comparing it to the output’s scriptPubKey. The accompanying tests verify that legitimate nested-singlesig change without redeem script is accepted, while adversarial cases (scriptPubKey pays a stranger, or bip32_derivation lists another key) raise PSBTOutputOwnershipContradictionError. Other wallet types paying this seed remain classified as spend.
Changed components
src/seedsigner/models/psbt_parser.pytests/test_psbt_parser.pyInspect captured patch +148 / −15
### src/seedsigner/models/psbt_parser.py
@@ -249,7 +249,7 @@ def parse(self):
_get_policy doesn't propagate cosigner errors, so two such policies match
without anything having tied them to the same keys. TODO: don't let a
policy with no cosigner information pass as a match between inputs.
- Outputs deliberately compare shape alone; see _policy_shape_matches.
+ Outputs deliberately compare shape alone; see _is_change_candidate.
5. _parse_outputs: organizes the output data (amounts, destination_addresses,
etc.) and verifies the ownership of the outputs that come back to this seed
@@ -411,7 +411,7 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
# Is this output change? If this output's policy is superficially similar to
# the spending wallet's policy (e.g. they're both 2-of-3 p2wsh), then it's a
# candidate for being change.
- if PSBTParser._policy_shape_matches(out_policy, self.policy):
+ if self._is_change_candidate(out, out_policy):
# Begin the extensive work to fully verify whether this output is indeed
# change.
@@ -708,23 +708,49 @@ def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache: dict | N
return policy
- @staticmethod
- def _policy_shape_matches(policy_a: dict, policy_b: dict) -> bool:
+ def _is_change_candidate(self, out: OutputScope, out_policy: dict) -> bool:
"""
- Compares two policies on the shape of the script they describe: the script type,
- plus m-of-n for multisig.
-
- A policy can also carry the cosigners resolved from the coordinator's global
- xpubs. Those are never authoritative here, and comparing them would let a psbt
- decide which of its own outputs get verified: one misannotated fingerprint makes
- that output's cosigners fail to resolve, and the output then stops matching the
- inputs' policy. Shape comes from the scriptPubKey and the supplied script, and the
- caller proves ownership rather than assuming it.
+ Determines whether an output is worth the full ownership check in _parse_outputs.
+
+ Returns True if the output's policy has the same "shape" as the inputs' policy:
+ the script type, plus m-of-n for multisig.
+
+ One outlier: Nested single sig (p2sh-p2wpkh). Its scriptPubKey is a p2sh hash of
+ its redeem script, but per BIP-174 the redeem script itself is optional.
+ When it is omitted, the output is superficially indistinguishable from plain p2sh.
+ If the inputs are p2sh-p2wpkh, then such an output would fail the policy
+ comparison test (p2sh != p2sh-p2wpkh) when it may have actually been possible to
+ verify it as our change.
+
+ So instead, when a p2sh output could be our own nested single sig change we let it
+ through and leave it to the rebuild process to verify if the output really is our
+ change.
+
+ Note: A multisig's input or output policy can also include the cosigners if
+ they're supplied in the global xpubs. But this function does not take the
+ cosigners into account; comparing the cosigners here would let a psbt decide which
+ of its own outputs get verified:
+ * One misannotated derivation path would make that output's cosigners fail
+ to resolve.
+ * The output's missing cosigners would mean that it would not match the inputs'
+ cosigners.
+ * End result: the output would not be considered a change candidate and would
+ not go through the same scrutiny that change candidates do.
+
+ Cosigner information, if provided, is evaluated later.
"""
+ # The outlier: a single sig p2sh output when the inputs are p2sh-p2wpkh.
+ if (
+ self.policy["type"] == "p2sh-p2wpkh" # Input is nested single sig
+ and out_policy["type"] == "p2sh" # Output parses as plain p2sh
+ and out.redeem_script is None # Output omits its redeem script
+ ):
+ return True
+
+ # All other outputs must have the same policy shape as the inputs
for field in ("type", "m", "n"):
- if policy_a.get(field) != policy_b.get(field):
+ if out_policy.get(field) != self.policy.get(field):
return False
-
return True
### tests/test_psbt_parser.py
@@ -2062,6 +2062,113 @@ def test__parse__rejects_a_multisig_output_whose_supplied_script_is_not_its_own(
self._parse(psbt)
+ def test__parse__counts_nested_single_sig_change_without_its_redeem_script_as_change(self):
+ """
+ A legitimate nested single sig (p2sh-p2wpkh) change output can omit its redeem
+ script (BIP-174 makes it optional) while still claiming (via bip32_derivations)
+ that a key owned by our seed will receive the change.
+
+ But the parser's proof of ownership check does not care about the missing redeem
+ script: the parser rebuilds p2sh(p2wpkh(K)) from the seed's own key at the claimed
+ path regardless. So the output should still be verifiable as change.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
+
+ # The output claims a single key and our seed really does derive it there.
+ assert len(psbt.outputs[0].bip32_derivations) == 1
+ public_key, derivation_path = list(psbt.outputs[0].bip32_derivations.items())[0]
+ assert PSBTParser.seed_owns_pubkey(self._root(), derivation_path.derivation, public_key, child_key_derivation_cache=None) is True
+
+ psbt.outputs[0].redeem_script = None
+
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 10_000
+ assert psbt_parser.spend_amount == 0
+
+
+ def test__parse__rejects_a_bare_p2sh_output_that_claims_this_seed_but_pays_someone_else(self):
+ """
+ Variation on the prior test: the redeem script is still omitted but this time the
+ psbt repoints its output at a stranger's p2sh-p2wpkh. Crucially, the output keeps
+ its claim on this seed, making this an attempt at deception (if there was no claim
+ on the output, it would simply be a typical external spend output).
+
+ When the parser rebuilds p2sh(p2wpkh(K)), the resulting scriptPubKey will not
+ match what the output commits to. The psbt should be refused with
+ PSBTOutputOwnershipContradictionError.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
+ psbt.outputs[0].redeem_script = None
+ psbt.outputs[0].script_pubkey = script.p2sh(script.p2wpkh(foreign_public_key()))
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__rejects_a_bare_p2sh_output_that_pays_this_seed_but_lists_another_key(self):
+ """
+ Another variation: the redeem script is omitted and the output still pays our key,
+ but its one derivation path entry lists another seed's key and fingerprint at the
+ same path.
+
+ The parser rebuilds p2sh(p2wpkh(K)) from our own key at that path and ignores the
+ fingerprint the entry lists. The result based on our key matches what the output
+ commits to, so it actually is our output even though the psbt claimed that it paid
+ to a different key. The psbt should be refused with
+ PSBTOutputOwnershipContradictionError.
+ """
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
+ psbt.outputs[0].redeem_script = None
+
+ # Swap the output's entry for another seed's key at the same path. The
+ # scriptPubKey still pays our key there.
+ derivation_path = list(psbt.outputs[0].bip32_derivations.values())[0]
+ path = bip32.path_to_str(derivation_path.derivation)
+ psbt.outputs[0].bip32_derivations.clear()
+ claim_seed_owns_key(psbt.outputs[0], path, foreign_public_key(path), seed=PSBTTestData.recipient_seed)
+
+ with pytest.raises(PSBTOutputOwnershipContradictionError):
+ self._parse(psbt)
+
+
+ def test__parse__counts_a_payment_to_another_wallet_of_this_seed_as_a_spend(self):
+ """
+ A psbt can pay another wallet of this seed, such as its native segwit account or a
+ multisig it belongs to, and annotate that output with our key. Each case below is
+ such a payment. Each should parse and be counted as a spend.
+
+ The parser makes an exception for nested single sig change that omits its redeem
+ script. An output must meet three criteria to qualify for the exception:
+ * inputs: the inputs are nested single sig
+ * output type: the output is parsed as plain p2sh
+ * redeem script: the output omits its redeem script
+
+ These outputs should be categorized as external spends if any of the three
+ criteria are not met. Each scenario in this test sets up one criterion to fail
+ while the other two are met.
+ """
+ # Fails the inputs condition: a native segwit input instead of nested single sig
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
+ psbt.outputs[0].redeem_script = None
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+ # Fails the output type condition: p2wpkh is not parsed as a plain p2sh output
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_CHANGE)
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+ # Fails the redeem script condition: legacy multisig is parsed as plain p2sh even
+ # when it supplies its redeem script.
+ psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE)
+ assert psbt.outputs[0].redeem_script is not None
+ psbt_parser = self._parse(psbt)
+ assert psbt_parser.change_amount == 0
+ assert psbt_parser.spend_amount == 10_000
+
+
def test_get_cosigners_returns_a_sorted_list(self):
"""
Two multisig scripts can list the same wallet's keys in different orders, so theWhy this scored 68/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.