fix WIF store ownership showing QR address
What the developer wrote
Authored by scgbckbone
fix WIF store ownership showing QR address
What changed, and why it matters
This commit fixes a bug in the COLDCARD hardware wallet where scanning or verifying a QR code for an address stored in the WIF (Wallet Import Format) store could display or encode the wrong QR address format. Previously, the code assumed all WIF-store addresses used the wallet's default address format, which could cause a mismatch when showing the QR code. The fix correctly tracks the address format for WIF-stored keys and adds a test to verify the QR code matches the expected address.
Review whether the QR mismatch could have caused users to share or verify an incorrectly encoded address, and consider issuing a firmware update note. The fix should be included in the next release; no immediate emergency response appears necessary.
Security signals we found
Incorrect address format used for QR display of WIF-stored keys
UI display inconsistency between shown address and QR-encoded address
Potential user confusion or misattribution of address ownership
Regression test added to prevent QR mismatch for WIF store addresses
Evidence from the diff
In shared/ownership.py, the OwnershipCache.search() method now returns a tuple (‘wif’, target_af) instead of the string ‘wif’ when an address is found in the WIF store. The caller, show_address(), now extracts target_af from this tuple and uses it to determine whether the QR code should be encoded as bech32/bech32m (Segwit/ Taproot) versus base58. Previously, the code referenced wallet.addr_fmt, which was undefined/invalid for WIF-store entries and could lead to incorrect QR encoding. A regression test was added in testing/test_ownership.py to capture the QR code and assert it matches the expected address for WIF-store entries.
Changed components
shared/ownership.pytesting/test_ownership.pyCOLDCARD WIF store address verification UIQR code display for WIF-stored single-sig addressesInspect captured patch +15 / −5
diff --git a/shared/ownership.py b/shared/ownership.py
index 7c3efb2..fce59f8 100644
--- a/shared/ownership.py
+++ b/shared/ownership.py
@@ -340,11 +340,12 @@ class OwnershipCache:
# nothing found among singlesig & registered multisig wallets
# check WIF store (single sig only)
if addr_fmt not in [AF_P2TR, AF_P2WSH]:
+ dis.fullscreen("WIF Store...")
from wif import iter_wif_store_addresses
target_af = AF_P2WPKH_P2SH if addr_fmt == AF_P2SH else addr_fmt
for i, store_addr in iter_wif_store_addresses(ch, target_af):
if store_addr == addr:
- return False, "wif", i+1
+ return False, ("wif", target_af), i+1
raise UnknownAddressExplained('Searched %d candidate addresses in %d wallet(s)'
' without finding a match.' % (c, len(matches)))
@@ -362,12 +363,14 @@ class OwnershipCache:
is_ms = isinstance(wallet, MultisigWallet)
msg = show_single_address(addr)
esc = ""
- if wallet == "wif":
+ if isinstance(wallet, tuple) and (wallet[0] == "wif"):
msg += '\n\nFound in WIF store at index %d' % subpath
+ addr_fmt = wallet[1]
else:
sp = wallet.render_path(*subpath)
msg += '\n\nFound in wallet:\n ' + wallet.name
msg += '\nDerivation path:\n ' + sp
+ addr_fmt = wallet.addr_fmt
if not is_ms:
esc = "0"
msg += "\n\nPress (0) to sign message with this key."
@@ -385,10 +388,10 @@ class OwnershipCache:
ch = await ux_show_story(msg, title=title, escape=esc, hint_icons=KEY_QR)
if ch in ("1"+KEY_QR):
await show_qr_code(addr, msg=addr, is_addrs=True,
- is_alnum=(wallet.addr_fmt & (AFC_BECH32 | AFC_BECH32M)))
+ is_alnum=(addr_fmt & (AFC_BECH32 | AFC_BECH32M)))
elif not is_ms and (ch == "0"): # only singlesig
from msgsign import sign_with_own_address
- await sign_with_own_address(sp, wallet.addr_fmt)
+ await sign_with_own_address(sp, addr_fmt)
else:
break
diff --git a/testing/test_ownership.py b/testing/test_ownership.py
index 8881d77..4f74e90 100644
--- a/testing/test_ownership.py
+++ b/testing/test_ownership.py
@@ -704,7 +704,7 @@ def test_named_wallet_search(wname, valid, method, clear_ms, import_ms_wallet, i
@pytest.mark.parametrize("idx", [1, 3])
def test_wif_store(addr_fmt, idx, is_q1, goto_home, pick_menu_item, scan_a_qr, cap_story, need_keypress,
src_root_dir, sim_root_dir, nfc_write, settings_remove, import_wif_to_store,
- load_shared_mod):
+ load_shared_mod, cap_screen_qr, press_cancel):
settings_remove("wifs")
@@ -752,5 +752,12 @@ def test_wif_store(addr_fmt, idx, is_q1, goto_home, pick_menu_item, scan_a_qr, c
title, story = cap_story()
assert addr == addr_from_display_format(story.split("\n\n")[0])
assert f"Found in WIF store at index {idx}" in story
+ need_keypress(KEY_QR if is_q1 else '1')
+ addr_qr = cap_screen_qr().decode()
+ if addr_fmt == "p2wpkh":
+ addr_qr = addr_qr.lower()
+
+ assert addr == addr_qr
+ press_cancel()
# EOF
Why this scored 43/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.