What changed, and why it matters
This commit fixes a bug in the COLDCARD hardware wallet firmware that prevented signing of 1-of-1 multisignature transactions. The change also hardens how the wallet detects single-key P2SH-wrapped SegWit inputs so it no longer misclassifies them as multisig when extra unrelated key paths are present. The bug could cause legitimate transactions to fail, but the patch also reduces the chance that a malformed PSBT could trick the device into treating a simple single-key input as a multisig one.
Users relying on 1-of-1 multisig or P2SH-P2WPKH should upgrade to a release containing this fix. Developers should review other PSBT classification paths for similar ordering bugs and ensure redeem-script structure, not subpath count, drives address-format detection.
Security signals we found
PSBT input classification bug fixed
1-of-1 multisig signing restored
P2SH-P2WPKH now requires exactly one BIP32 subpath
Malformed PSBT with extra BIP32 paths now rejected for P2SH-P2WPKH
New regression tests added
Evidence from the diff
In shared/psbt.py, the input classification logic was reordered. Previously, the code guessed which key to use based on the number of BIP32 subpaths before deciding whether the redeem script was P2SH-P2WPKH. A 1-of-1 multisig PSBT has exactly one subpath, so it was misclassified as P2SH-P2WPKH and signing failed. The patch now checks the redeem script structure first: if it is the 22-byte P2SH-P2WPKH template (OP_0 + 20-byte witness program), it requires exactly one subpath and treats it as single-key; otherwise it treats the input as multisig. A new assertion rejects P2SH-P2WPKH PSBTs that include multiple BIP32 paths. Tests were added for 1-of-1 multisig signing and for rejecting multi-path P2SH-P2WPKH PSBTs.
Changed components
shared/psbt.pyCOLDCARD transaction signing flowP2SH-P2WPKH and 1-of-1 multisig handlingInspect captured patch +45 / −20
diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md
index 9c5afe3..0952721 100644
--- a/releases/Next-ChangeLog.md
+++ b/releases/Next-ChangeLog.md
@@ -41,6 +41,7 @@ This lists the new changes that have not yet been published in a normal release.
- Bugfix: Handle malformed NDEF records robustly. Thanks, @Damir
- Bugfix: Ignore `bkpw` if added to backup. Thanks [@dmonakhov](https://github.com/dmonakhov)
- Bugfix: Keep NFC export tag live for repeated probes
+- Bugfix: Fix 1of1 multisig signing failure
# Mk Specific Changes
diff --git a/shared/psbt.py b/shared/psbt.py
index 7e66d57..599637f 100644
--- a/shared/psbt.py
+++ b/shared/psbt.py
@@ -829,17 +829,20 @@ class psbtInputProxy(psbtProxy):
self.scriptSig = redeem_script
- # new cheat: psbt creator probably telling us exactly what key
- # to use, by providing exactly one. This is ideal for p2sh wrapped p2pkh
- if len(subpaths) == 1:
+ if not addr_is_segwit and len(redeem_script) == 22 and \
+ redeem_script[0] == 0 and redeem_script[1] == 20:
+ # segwit p2pkh wrapped in p2sh: exactly one key, not multisig.
+ # psbt creator tells us the key by providing exactly one subpath.
+ self.addr_fmt = AF_P2WPKH_P2SH
+ addr = redeem_script[2:22]
+ self.is_segwit = True
+
+ assert len(subpaths) == 1, "p2sh-p2wpkh needs one key"
which_key, = subpaths.keys()
else:
- # Assume we'll be signing with any key we know
- # - limitation: we cannot be two legs of a multisig (only if CCC feature used)
- # - but if partial sig already in place, ignore that one
- if not which_key:
- which_key = set()
+ self.is_multisig = True
+ which_key = set()
for pubkey, path in subpaths.items():
if self.part_sigs and (pubkey in self.part_sigs):
# pubkey has already signed, so ignore
@@ -850,20 +853,8 @@ class psbtInputProxy(psbtProxy):
which_key.add(pubkey)
elif pubkey in psbt.wif_store:
- # maybe sset some input value
which_key.add(pubkey)
- if not addr_is_segwit and \
- len(redeem_script) == 22 and \
- redeem_script[0] == 0 and redeem_script[1] == 20:
- # it's actually segwit p2pkh inside p2sh
- self.addr_fmt = AF_P2WPKH_P2SH
- addr = redeem_script[2:22]
- self.is_segwit = True
- else:
- # multiple keys involved
- self.is_multisig = True
-
if self.witness_script and not self.is_segwit and self.is_multisig:
# bugfix
self.addr_fmt = AF_P2WSH_P2SH
diff --git a/testing/test_multisig.py b/testing/test_multisig.py
index 98a183e..14e02a3 100644
--- a/testing/test_multisig.py
+++ b/testing/test_multisig.py
@@ -1510,6 +1510,24 @@ def test_ms_sign_simple(M_N, num_ins, dev, addr_fmt, clear_ms, incl_xpubs, impor
else:
try_sign(psbt)
+
+@pytest.mark.parametrize("finalize", [True, False])
+def test_1of1_multisig_sign(finalize, clear_ms, import_ms_wallet, fake_ms_txn, start_sign,
+ end_sign, cap_story):
+ # Minimal 1-of-1 multisig: import the wallet, then sign a 1-in/1-out PSBT.
+ clear_ms()
+ M = N = 1
+ keys = import_ms_wallet(M, N, accept=True)
+
+ psbt = fake_ms_txn(1, 1, M, keys)
+
+ start_sign(psbt, finalize=finalize)
+ title, story = cap_story()
+ assert title == "OK TO SEND?"
+
+ end_sign(accept=True, finalize=finalize)
+
+
@pytest.mark.unfinalized
@pytest.mark.bitcoind
@pytest.mark.parametrize('num_ins', [ 15 ])
diff --git a/testing/test_sign.py b/testing/test_sign.py
index 1c93284..1c0f387 100644
--- a/testing/test_sign.py
+++ b/testing/test_sign.py
@@ -1664,6 +1664,21 @@ def test_wrong_pubkey(dev, try_sign, fake_txn):
msg = ee.value.args[0]
assert ('pubkey vs. address wrong' in msg)
+
+def test_p2sh_p2wpkh_multiple_bip32_paths_rejected(fake_txn, start_sign, cap_story):
+ psbt = fake_txn(1, 1, segwit_in=True, wrapped=True)
+ po = BasicPSBT().parse(psbt)
+
+ other = BIP32Node.from_master_secret(os.urandom(32))
+ other_key = other.subkey_for_path("0/0")
+ po.inputs[0].bip32_paths[other_key.sec()] = other.fingerprint() + struct.pack('<II', 0, 0)
+
+ start_sign(po.as_bytes(), finalize=True)
+ title, story = cap_story()
+ assert title == "Failure"
+ assert "p2sh-p2wpkh needs one key" in story
+
+
def test_incomplete_signing(dev, try_sign, fake_txn, cap_story):
# psbt where we only sign one input
# - must not allow finalization
Why this scored 42/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.