From 59eb529a203806a3f90d08d653ec778392a66dd1 Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Tue, 26 May 2026 15:41:08 +0200 Subject: [PATCH] Reject witness-only UTXO for legacy inputs; Suppress fee for unverified witness UTXOs;normalize legacy inputs to proper utxo --- releases/Next-ChangeLog.md | 3 + shared/psbt.py | 88 +++++++++++++++++----------- testing/psbt.py | 36 ++++++++++++ testing/test_multisig.py | 45 ++++++--------- testing/test_sign.py | 115 +++++++++++++++++++++++++++++++++++-- testing/test_teleport.py | 2 +- 6 files changed, 224 insertions(+), 65 deletions(-) diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index 5951180a..ba339b6b 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -9,6 +9,9 @@ This lists the new changes that have not yet been published in a normal release. - Enhancement: WIF Store export watch-only descriptor - Enhancement: WIF Store address detection without the need for PSBT_IN_BIP32_DERIVATION (Electrum support) - Enhancement: Improve USB length validation +- Bugfix: Fixes legacy input amount spoofing by rejecting witness-utxo-only PSBT inputs when Coldcard is expected to sign a non-segwit input. + When both UTXO fields are present the full non_witness_utxo is now preferred for amount/script lookup. Thanks, @Damir +- Bugfix: Emit warning and do not calculate fee for legacy UTXOs with only witness utxo - Bugfix: Disable Virtual Disk and NFC before activating HSM - Bugfix: Custom address default menu position wrong - Bugfix: Delta Mode Trick PIN was never restored from backup diff --git a/shared/psbt.py b/shared/psbt.py index 599637f8..d9eb5861 100644 --- a/shared/psbt.py +++ b/shared/psbt.py @@ -720,49 +720,58 @@ class psbtInputProxy(psbtProxy): fd = self.fd old_pos = fd.tell() - if self.witness_utxo: - # Going forward? Just what we will witness; no other junk - # - prefer this format, altho does that imply segwit txn must be generated? - # - I don't know why we wouldn't always use this - # - once we use this partial utxo data, we must create witness data out + if self.utxo: + # skip over all the parts of the txn we don't care about, without + # fully parsing it... pull out a single TXO + fd.seek(self.utxo[0]) + + _, marker, flags = unpack("= 0, "negative input value: i%d" % i total_in += utxo.nValue + if not inp.utxo and not inp.witness_utxo_is_provably_segwit(utxo): + unverified_witness_utxo.append(i) # Look at what kind of input this will be, and therefore what # type of signing will be required, and which key we need. # - also validates redeem_script when present # - also finds appropriate multisig wallet to be used inp.determine_my_signing_key(i, utxo, self.my_xfp, self, cosign_xfp) + + if inp.required_key and not inp.is_segwit and not inp.utxo: + raise FatalPSBTIssue('Legacy input #%d requires non-witness UTXO' % i) + # determine_my_signing_key is updating fully_signed for multisig inputs # based on redeem/witness script if inp.fully_signed: @@ -1898,7 +1914,7 @@ class psbtObject(psbtProxy): # XXX scan witness data provided, and consider those ins signed if not multisig? - if not foreign: + if not foreign and not unverified_witness_utxo: # no foreign inputs, we can calculate the total input value self.total_value_in = total_in assert total_in > 0 or self.por322, "zero value txn" @@ -1907,9 +1923,15 @@ class psbtObject(psbtProxy): # OK for multi-party transactions (coinjoin etc.) assert not self.por322 # cannot have foreign inputs in POR txn self.total_value_in = None - self.warnings.append( - ("Unable to calculate fee", "Some input(s) haven't provided UTXO(s): " + seq_to_str(foreign)) - ) + if foreign: + self.warnings.append( + ("Unable to calculate fee", "Some input(s) haven't provided UTXO(s): " + seq_to_str(foreign)) + ) + if unverified_witness_utxo: + self.warnings.append( + ("Unable to calculate fee", "Some input(s) provided unverified witness UTXO(s): " + + seq_to_str(unverified_witness_utxo)) + ) if len(self.presigned_inputs) == self.num_inputs: # Maybe wrong for multisig cases? Maybe they want to add their diff --git a/testing/psbt.py b/testing/psbt.py index ab7912a9..c893a7ce 100644 --- a/testing/psbt.py +++ b/testing/psbt.py @@ -517,6 +517,42 @@ class BasicPSBT: def as_b64_str(self): return b64encode(self.as_bytes()).decode() + def convert_witness_utxo_to_utxo(self, idx): + # Test helper: the original prev txn cannot be reconstructed from a + # witness_utxo, so retarget this PSBT input to a synthetic funding txn. + inp = self.inputs[idx] + assert inp.witness_utxo + assert inp.utxo is None + + prev_txo = CTxOut() + prev_txo.deserialize(io.BytesIO(inp.witness_utxo)) + + if self.is_v2(): + assert inp.prevout_idx is not None + prevout_idx = inp.prevout_idx + else: + assert self.parsed_txn + txin = self.parsed_txn.vin[idx] + prevout_idx = txin.prevout.n + + funding = CTransaction() + funding.nVersion = 2 + funding.vin = [CTxIn(COutPoint(0, 0xffffffff), nSequence=0xffffffff)] + funding.vout = [CTxOut(0, b'') for _ in range(prevout_idx)] + funding.vout.append(prev_txo) + funding.calc_sha256() + + inp.utxo = funding.serialize_with_witness() + inp.witness_utxo = None + + if self.is_v2(): + inp.previous_txid = ser_uint256(funding.sha256) + else: + txin.prevout.hash = funding.sha256 + self.txn = self.parsed_txn.serialize_with_witness() + + return funding + def to_v2(self): if self.version is None or self.version == 0: self.version = 2 diff --git a/testing/test_multisig.py b/testing/test_multisig.py index 14e02a39..94d5890d 100644 --- a/testing/test_multisig.py +++ b/testing/test_multisig.py @@ -1279,7 +1279,7 @@ def fake_ms_txn(pytestconfig): # - but has UTXO's to match needs from struct import pack - def doit(num_ins, num_outs, M, keys, fee=10000, outvals=None, segwit_in=False, + def doit(num_ins, num_outs, M, keys, fee=10000, outvals=None, outstyles=['p2pkh'], change_outputs=[], incl_xpubs=False, hack_psbt=None, hack_change_out=False, input_amount=1E8, psbt_v2=None, bip67=True, violate_script_key_order=False, path_mapper=None, inp_af=AF_P2WSH, @@ -1329,21 +1329,14 @@ def fake_ms_txn(pytestconfig): ) # lots of supporting details needed for p2sh inputs - if inp_af: - if inp_af == AF_P2WSH: - psbt.inputs[i].witness_script = script - elif inp_af == AF_P2SH: - psbt.inputs[i].redeem_script = script - else: - assert inp_af == AF_P2WSH_P2SH - psbt.inputs[i].witness_script = script - psbt.inputs[i].redeem_script = b'\0\x20' + sha256(script).digest() - + if inp_af == AF_P2WSH: + psbt.inputs[i].witness_script = script + elif inp_af == AF_P2SH: + psbt.inputs[i].redeem_script = script else: - if segwit_in: - psbt.inputs[i].witness_script = script - else: - psbt.inputs[i].redeem_script = script + assert inp_af == AF_P2WSH_P2SH + psbt.inputs[i].witness_script = script + psbt.inputs[i].redeem_script = b'\0\x20' + sha256(script).digest() for pubkey, xfp_path in details: psbt.inputs[i].bip32_paths[pubkey] = b''.join(pack('