Match policy types exactly and refuse an unsupported one
What changed, and why it matters
This commit fixes a bug in how SeedSigner parses Bitcoin transaction outputs. Previously, the code guessed script types using partial string matches (like checking if 'pkh' appears anywhere in the type name) and had no fallback for unknown types. A specially crafted transaction using an unsupported script type could slip past the safety checks and crash the device with a confusing error, or potentially be misclassified as change. The patch now matches script types exactly and explicitly rejects any unsupported type with a clear error message.
Review and merge the patch. Ensure the new exact-match policy type handling covers all supported wallet configurations. Consider adding additional tests for other unsupported script types and verifying that the RuntimeError is handled appropriately by upstream UI code to avoid user confusion.
Security signals we found
CWE-20: Improper Input Validation - substring matching allowed ambiguous/unsupported script type handling
CWE-248: Uncaught Exception - unsupported script types caused bare TypeError instead of controlled failure
CWE-754: Improper Check for Unusual or Exceptional Conditions - missing else branch for unsupported policy types
Defense in depth: explicit allowlist of supported script types with catch-all rejection
Crash/DoS vector: malformed PSBT could cause unhandled exception during transaction parsing
Evidence from the diff
In psbt_parser.py’s _parse_outputs(), the rebuild chain previously matched single-sig and taproot policies via substring tests (‘pkh’ in type, ‘p2tr’ in type) and lacked an else branch. Because embit reports unrecognized script types as None, and _get_policy passes None through, a PSBT with both inputs and outputs using an unrecognized script type would pass the shape gate (None == None), reach the substring test, and raise a bare TypeError. The patch replaces substring matching with exact equality checks against supported types (p2pkh, p2sh-p2wpkh, p2wpkh, p2tr) and adds a catch-all else raising RuntimeError with the unsupported type. It also removes an inner guard that could no longer be reached and updates a downstream condition to use exact type comparison. A test exercises this with a p2pk PSBT.
Changed components
src/seedsigner/models/psbt_parser.py_parse_outputs() methodPolicy type detection and scriptPubKey rebuild logictests/test_psbt_parser.pyInspect captured patch +38 / −11
### src/seedsigner/models/psbt_parser.py
@@ -421,9 +421,8 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
multisig_script = out.redeem_script
rebuilt_script_pubkey = script.p2sh(multisig_script)
- # single-sig: p2pkh, p2sh-p2wpkh, and p2wpkh; taproot handled separately
- # below.
- elif "pkh" in self.policy["type"]:
+ # single-sig; taproot handled separately below.
+ elif self.policy["type"] in ("p2pkh", "p2sh-p2wpkh", "p2wpkh"):
# Sanity check; a single sig output shouldn't have multiple derivation
# paths.
if len(out.bip32_derivations) > 1:
@@ -439,7 +438,7 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
# considered an external spend.
pass
- elif "p2tr" in self.policy["type"]:
+ elif self.policy["type"] == "p2tr":
taproot_entries = list(out.taproot_bip32_derivations.values())
if len(taproot_entries) == 0:
@@ -472,6 +471,12 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
# TODO: Support keys in script tree leaves
pass
+ else:
+ # Safety catch-all: any new script types will need explicit handling
+ # above. Note that embit reports unrecognized script types as `None`,
+ # which is also caught here.
+ raise RuntimeError(f"Unsupported policy type: {self.policy['type']}")
+
verified_derivation_path = self.verified_output_derivation_paths[i]
if rebuilt_script_pubkey.data == vout[i].script_pubkey.data:
@@ -548,13 +553,7 @@ def _parse_outputs(self, child_key_derivation_cache: dict):
# their "known-good" multisig descriptor.
is_change = True
- else:
- # No handler claimed a matching output, which the branches above
- # should make impossible. Raise rather than leave is_change True;
- # that would record change with nothing verified behind it.
- raise RuntimeError(f"Output {i} matched but no verification handler applies")
-
- elif verified_derivation_path is not None and "p2tr" not in self.policy["type"]:
+ elif verified_derivation_path is not None and self.policy["type"] != "p2tr":
# The psbt claims one of this seed's keys on this output, yet the
# output does NOT pay what that claim describes. We treat this
# deception as an attack.
### tests/test_psbt_parser.py
@@ -1888,3 +1888,31 @@ def test__parse__rejects_a_multisig_output_whose_supplied_script_is_not_its_own(
with pytest.raises(PSBTOutputOwnershipContradictionError):
self._parse(psbt)
+
+ def test__parse__refuses_an_unsupported_script_type(self):
+ """
+ Parsing should be aborted if a psbt has inputs and outputs that use a script type
+ that embit doesn't recognize. embit reports unhandled types as None. In this case
+ the output's policy shape would match the input's (`None` == `None`) and so the
+ parse would consider it as possible change. Rather than continue, we expect the
+ catch-all safety check to spot the unsupported script type and raise RuntimeError.
+ """
+ root = self._root()
+
+ # embit does not support p2pk so its script type will be `None`
+ def p2pk(public_key: PublicKey) -> script.Script:
+ OP_PUSHBYTES_33 = b"\x21"
+ OP_CHECKSIG = b"\xac"
+ return script.Script(OP_PUSHBYTES_33 + public_key.sec() + OP_CHECKSIG)
+
+ psbt = self._psbt_with_change()
+ psbt.inputs[0].witness_utxo.script_pubkey = p2pk(root.derive("m/84h/1h/0h/0/3").get_public_key())
+ psbt.outputs[0].script_pubkey = p2pk(root.derive("m/84h/1h/0h/1/0").get_public_key())
+
+ # The policy shape check really does let this output through to the scriptPubKey
+ # rebuild step.
+ assert psbt.inputs[0].witness_utxo.script_pubkey.script_type() is None
+ assert psbt.outputs[0].script_pubkey.script_type() is None
+
+ with pytest.raises(RuntimeError, match="Unsupported policy type"):
+ self._parse(psbt)Why this scored 60/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.