SFT-8169: remove dead witness iterator branch and reject witness serialized unsigned txns clearly
What changed, and why it matters
This commit cleans up how the Passport hardware wallet handles Bitcoin transactions that incorrectly include witness data. It removes a dead, broken code branch that referenced non-existent variables and replaces a confusing low-level error with a clear user-facing message. The same invalid transactions were already being rejected; this change just makes the rejection clearer and safer, and adds tests to prove it.
No urgent action required. This is a defensive cleanup that improves robustness and error clarity. Users and integrators should ensure firmware is updated to include this change, and continue to follow BIP-174 compliant PSBT serialization.
Security signals we found
Removes unreachable branch referencing undefined variables (`fd`, `num_in`), eliminating a latent crash or confusion risk
Replaces a bare ValueError with a structured FatalPSBTIssue, improving error handling and user messaging
Maintains existing rejection policy for BIP-174 non-compliant PSBTs (no validation relaxation)
Adds unit tests covering witness-serialized unsigned transaction rejection and edge cases
Evidence from the diff
The patch removes the had_witness and wit_start state from psbtObject and simplifies input_witness_iter() to always yield placeholder witnesses. It changes parse_txn() to explicitly detect segwit-serialized unsigned transactions (marker==0 and flags!=0) and raise FatalPSBTIssue('Unsigned transaction must not include witness data') instead of relying on _skip_n_objs() raising a bare ValueError('CTxInWitness'). finalize() now decides whether to include witness data based solely on whether any input is segwit, not on whether the original transaction had witness data. A unit test is added to verify compliant parsing, rejection of witness-serialized unsigned transactions, zero-input rejection, version checks, and placeholder witness iteration.
Changed components
ports/stm32/boards/Passport/modules/psbt.pyports/stm32/boards/Passport/modules/tests/test_unit.pyports/stm32/boards/Passport/modules/tests/unit/psbt_unsigned_txn.pyInspect captured patch +112 / −32
### ports/stm32/boards/Passport/modules/psbt.py
@@ -990,12 +990,10 @@ def __init__(self):
# details that we discover as we go
self.inputs = None
self.outputs = None
- self.had_witness = None
self.num_inputs = None
self.num_outputs = None
self.vin_start = None
self.vout_start = None
- self.wit_start = None
self.txn_version = None
self.lock_time = None
self.total_value_out = None
@@ -1079,13 +1077,18 @@ def parse_txn(self):
# don't force that
self.txn_version, marker, flags = unpack("<iBB", fd.read(6))
- self.had_witness = (marker == 0 and flags != 0x0)
assert self.txn_version in {1, 2}, "bad txn version"
- if not self.had_witness:
- # rewind back over marker+flags
- fd.seek(-2, 1)
+ # BIP-174 requires the unsigned transaction to be serialized without witness
+ # data. A zero marker can only be the segwit marker here, since an input count
+ # of zero is rejected below. This already failed, but as a bare ValueError from
+ # _skip_n_objs(), which has no pattern for 'CTxInWitness'.
+ if marker == 0 and flags != 0x0:
+ raise FatalPSBTIssue('Unsigned transaction must not include witness data')
+
+ # rewind back over marker+flags
+ fd.seek(-2, 1)
num_in = deser_compact_size(fd)
assert num_in > 0, "no ins?"
@@ -1104,13 +1107,6 @@ def parse_txn(self):
end_pos = sum(self.txn)
- # remainder is the witness data, and then the lock time
-
- if self.had_witness:
- # we'll need to come back to this pos if we
- # want to read the witness data later.
- self.wit_start = _skip_n_objs(fd, num_in, 'CTxInWitness')
-
# we are at end of outputs, and no witness data, so locktime is here
self.lock_time = unpack("<I", fd.read(4))[0]
@@ -1142,23 +1138,10 @@ def input_iter(self):
fd.seek(cont)
def input_witness_iter(self):
- # yield all the witness data, in order by input
- if not self.had_witness:
- # original txn had no witness data, so provide placeholder objs
- for in_idx in range(self.num_inputs):
- yield in_idx, CTxInWitness()
- return
-
- fd.seek(self.wit_start)
- for idx in range(num_in):
-
- wit = CTxInWitness()
- wit.deserialize(fd)
-
- cont = fd.tell()
- yield idx, wit
-
- fd.seek(cont)
+ # yield a placeholder witness for each input: parse_txn() rejects a witness
+ # serialized unsigned txn, so there is never any witness data to preserve
+ for in_idx in range(self.num_inputs):
+ yield in_idx, CTxInWitness()
def guess_M_of_N(self):
# Peek at the inputs to see if we can guess M/N value. Just takes
@@ -1867,9 +1850,8 @@ def finalize(self, fd):
fd.write(pack('<i', self.txn_version)) # nVersion
# does this txn require witness data to be included?
- # - yes, if the original txn had some
# - yes, if we did a segwit signature on any input
- needs_witness = self.had_witness or any(i.is_segwit for i in self.inputs if i)
+ needs_witness = any(i.is_segwit for i in self.inputs if i)
if needs_witness:
# zero marker, and flags=0x01
### ports/stm32/boards/Passport/modules/tests/test_unit.py
@@ -48,6 +48,10 @@ def test_hdnode_blank(test):
assert test('hdnode_blank.py') == b'OK'
+def test_psbt_unsigned_txn(test):
+ assert test('psbt_unsigned_txn.py') == b'OK'
+
+
def test_psbt_multisig_approval(test):
assert test('psbt_multisig_approval.py') == b'OK'
### ports/stm32/boards/Passport/modules/tests/unit/psbt_unsigned_txn.py
@@ -0,0 +1,94 @@
+# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. <hello@foundation.xyz>
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+# BIP-174 requires the unsigned transaction inside a PSBT to be serialized without
+# witness data. Passport rejects anything else, so it never has witness data of its
+# own to preserve and always fills the witness area itself when finalizing.
+
+from uio import BytesIO
+from ustruct import pack
+
+from exceptions import FatalPSBTIssue
+from psbt import psbtObject
+from serializations import COutPoint, CTxIn, CTxInWitness, CTxOut, ser_compact_size
+
+P2WPKH_SCRIPT = b'\x00\x14' + (b'\x11' * 20)
+OUTPUT_VALUE = 1000
+
+
+class FakePSBT:
+ '''Just enough of a psbtObject for parse_txn() and the iterators to work on.'''
+
+ def __init__(self, raw):
+ self.fd = BytesIO(raw)
+ self.txn = (0, len(raw))
+ self.total_value_out = None
+
+
+def ser_unsigned_txn(num_in=1, num_out=1, version=2, witness=False):
+ body = pack('<i', version)
+
+ if witness:
+ body += b'\x00\x01'
+
+ body += ser_compact_size(num_in)
+ for idx in range(num_in):
+ body += CTxIn(COutPoint(idx + 1, idx)).serialize()
+
+ body += ser_compact_size(num_out)
+ for _ in range(num_out):
+ body += CTxOut(OUTPUT_VALUE, P2WPKH_SCRIPT).serialize()
+
+ if witness:
+ for _ in range(num_in):
+ body += CTxInWitness().serialize()
+
+ body += pack('<I', 0)
+ return body
+
+
+def must_raise(exc_type, call):
+ try:
+ call()
+ except exc_type:
+ return
+
+ raise RuntimeError('expected {}'.format(exc_type.__name__))
+
+
+# A compliant unsigned transaction parses, and the positions it records let the
+# input and output iterators walk it.
+psbt = FakePSBT(ser_unsigned_txn(num_in=2, num_out=3))
+psbtObject.parse_txn(psbt)
+
+assert psbt.txn_version == 2
+assert psbt.num_inputs == 2
+assert psbt.num_outputs == 3
+assert psbt.lock_time == 0
+
+assert [idx for idx, _ in psbtObject.input_iter(psbt)] == [0, 1]
+assert [txo.nValue for _, txo in psbtObject.output_iter(psbt)] == [OUTPUT_VALUE] * 3
+
+# Witness serialization is rejected with a message a caller can show, rather than
+# the bare ValueError('CTxInWitness') that _skip_n_objs() used to raise once the
+# parser reached the witness area.
+must_raise(FatalPSBTIssue,
+ lambda: psbtObject.parse_txn(FakePSBT(ser_unsigned_txn(witness=True))))
+
+# A zero input count is indistinguishable from the segwit marker, so it is caught on
+# that path rather than by the 'no ins?' assertion. Either way it is rejected.
+must_raise(FatalPSBTIssue,
+ lambda: psbtObject.parse_txn(FakePSBT(ser_unsigned_txn(num_in=0))))
+
+# Version checking is unaffected.
+must_raise(AssertionError,
+ lambda: psbtObject.parse_txn(FakePSBT(ser_unsigned_txn(version=3))))
+
+# finalize() fills the witness area itself, so every input gets a fresh empty
+# witness it can assign a stack to.
+witnesses = list(psbtObject.input_witness_iter(psbt))
+assert [idx for idx, _ in witnesses] == [0, 1]
+assert all(wit.scriptWitness.stack == [] for _, wit in witnesses)
+assert witnesses[0][1] is not witnesses[1][1]
+
+return_value.write(b'OK')Why this scored 30/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.