Harden BIP322 POR foreign-input check
What the developer wrote
Authored by scgbckbone
Harden BIP322 POR foreign-input check
Gate on required_key instead of num_our_keys: keypath metadata is
attacker-controlled, and the zero-xfp placeholder is rewritten to our
fingerprint on the fly, so a foreign input carrying a forged keypath
plus a partial signature looked both 'ours' and already-signed, and
was never rejected nor signed - while the SIGHASH_ALL signature over
input zero still committed to its prevout.
required_key is only set after the input's pubkey/scripts validate
against our seed or WIF store, so gating on it fails closed for all
three variants: no keypaths, forged keypaths, and presigned inputs.
Add regression test with a foreign UTXO (different seed) carrying a
zero-xfp keypath and a garbage partial signature.
What changed, and why it matters
This commit fixes a bug in COLDCARD's BIP-322 Proof of Reserves (POR) feature. POR lets a wallet prove it controls certain bitcoins without moving them. The bug allowed an attacker to include someone else's unspent coin in the proof by disguising it with fake key-path metadata and a bogus partial signature. The device would then treat that foreign input as already signed and belonging to the wallet, making the proof falsely claim control of funds the wallet does not actually own. The fix checks that the device can actually derive the required signing key for each input, rather than relying on attacker-controlled key-path data.
Users relying on BIP-322 Proof of Reserves should upgrade to a release containing this fix before trusting POR assertions. Review any past POR signatures for unexpected inputs if the signer ran vulnerable firmware. Developers should ensure similar checks use cryptographically validated required_key rather than attacker-controlled metadata.
Security signals we found
Bypass of foreign-input rejection in BIP-322 Proof of Reserves
Attacker-controlled PSBT keypath metadata used to spoof wallet ownership
Partial signature used to make foreign input appear already signed
Fix gates on required_key (cryptographic validation) instead of num_our_keys (metadata matching)
Regression test demonstrates exploit with foreign seed-derived key and zero-XFP keypath
Evidence from the diff
In shared/psbt.py, the POR foreign-input check previously gated on inp.num_our_keys, which is set based on matching the PSBT’s bip32 keypaths against the device’s fingerprint. Because the zero-XFP placeholder is rewritten to the device’s fingerprint on the fly, and because a partial signature makes an input appear already signed, a foreign UTXO with a forged keypath plus garbage partial signature would pass the old check. The patch changes the gate to inp.required_key, which is only populated after the input’s pubkey/scripts successfully validate against the device’s seed or WIF store. This closes the bypass for no-keypath, forged-keypath, and presigned-input variants. A regression test is added in testing/test_bip322.py.
Changed components
shared/psbt.pyBIP-322 Proof of Reserves signing flowPSBT input validation / consider_inputs()Inspect captured patch +27 / −3
### releases/Next-ChangeLog.md
@@ -4,7 +4,8 @@ This lists the new changes that have not yet been published in a normal release.
# Shared Improvements - Both Mk and Q
-- Bugfix: Reject foreign inputs from BIP-322 Proof of Reserves.
+- Bugfix: Reject foreign inputs from BIP-322 Proof of Reserves, including inputs
+ disguised with forged key-path metadata or partial signatures.
- Bugfix: Restore the ability to view the device-generated seed before adding user
entropy, which was available in the previous dice-roll workflow but was inadvertently
removed in 5.6.1/1.5.1Q. The new **View TRNG Words** menu item displays the full
### shared/psbt.py
@@ -1885,7 +1885,11 @@ def consider_inputs(self, cosign_xfp=None):
# - also finds appropriate multisig wallet to be used
inp.determine_my_signing_key(i, utxo, self.my_xfp, self, cosign_xfp)
- if self.por322 and i and not inp.num_our_keys:
+ if self.por322 and i and not inp.required_key:
+ # every POR input past to_spend must be one we will actually sign;
+ # num_our_keys is not enough: forged keypaths (incl. zero-xfp
+ # placeholder) plus a partial sig would make a foreign input
+ # look ours and already-signed (required_key is None)
foreign_por = True
if inp.required_key and not inp.is_segwit and not inp.utxo:
### testing/test_bip322.py
@@ -3,7 +3,7 @@
# BIP-322 Message Signing and Proof of Reserves
# NOTE: Run this module with and without --psbt2 to cover both PSBT versions.
#
-import pytest, time, os
+import pytest, time, os, struct
from io import BytesIO
from decimal import Decimal
from constants import SIGHASH_MAP, AF_P2SH, AF_P2WSH, AF_P2WSH_P2SH
@@ -338,6 +338,25 @@ def hack(psbt_in):
assert "Foreign inputs not allowed in BIP-322 Proof of Reserves" in story
+def test_bip322_por_presigned_foreign_input(bip322_txn, start_sign, cap_story):
+ # Foreign UTXO (key from a different seed) carrying a forged zero-xfp keypath
+ # - rewritten to our master fingerprint on the fly - plus a partial signature,
+ # so the input looks "ours" and already signed; must still be rejected.
+ foreign_sec = BIP32Node.from_master_secret(b'\x77' * 32).subkey_for_path("0/0").sec()
+
+ def hack(psbt_in):
+ inp = psbt_in.inputs[1]
+ inp.bip32_paths = {foreign_sec: b"\x00" * 4 + struct.pack("<I", 0)}
+ inp.part_sigs[foreign_sec] = b"\x30" + 70 * b"a"
+
+ psbt, _ = bip322_txn([["p2wpkh", None, None], ["p2wpkh", None, 10000000]],
+ witness_utxo=[1], psbt_hacker=hack)
+ start_sign(psbt)
+ title, story = cap_story()
+ assert title == "Failure"
+ assert "Foreign inputs not allowed in BIP-322 Proof of Reserves" in story
+
+
def test_bip322_por_input0_bip32_paths_required(bip322_txn, start_sign, cap_story):
def hack(psbt_in):
psbt_in.inputs[0].bip32_paths = NoneWhy this scored 76/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.