What the developer wrote
Authored by Peter D. Gray
nits
What changed, and why it matters
This tiny code change fixes a likely bug in the COLDCARD's NFC feature. Previously, after an NFC operation finished and the device wiped sensitive data, the code continued running in a loop instead of stopping. The added 'break' makes it exit the loop immediately after wiping. This could prevent repeated or unnecessary NFC transmissions of sensitive wallet data, reducing the chance of leaking secrets or leaving the device in an unexpected state.
Review the full NFC session lifecycle to confirm no other paths continue execution after wipe, and verify the break does not skip necessary cleanup. Consider whether this bug warrants a security advisory if sensitive data could be re-transmitted.
Security signals we found
Loop continues after sensitive-data wipe without termination
NFC handler processes wallet secrets
Added break prevents repeated post-wipe execution
Evidence from the diff
In shared/nfc.py, a ‘break’ was added after ‘await self.wipe(…)’ inside a loop in an NFC method. Without the break, execution would continue iterating after the wipe, potentially re-entering logic that should only run once per completed NFC session. The patch also includes minor style cleanups (parentheses, whitespace). The security relevance is inferred from context: NFC on a hardware wallet handles sensitive data, and failing to exit after a wipe could cause repeated exposure or state confusion. No vendor security disclosure or researcher attribution is present in the commit.
Changed components
shared/nfc.pyNFCHandler NFC session completion/wipe pathInspect captured patch +4 / −2
diff --git a/shared/nfc.py b/shared/nfc.py
index b01d4bc..b06b6f4 100644
--- a/shared/nfc.py
+++ b/shared/nfc.py
@@ -107,13 +107,14 @@ class NFCHandler:
from glob import dis
here = bytes(256)
end = 8196
- for pos in range(0, end, 256) :
+ for pos in range(0, end, 256):
self.i2c.writeto_mem(I2C_ADDR_USER, pos, here, addrsize=16)
- if pos == 256 and not full_wipe: break
+ if (pos == 256) and not full_wipe: break
# 6ms per 16 byte row, worst case, so ~100ms here per iter! 3.2seconds total
if full_wipe:
dis.progress_bar_show(pos / end)
+
await self.wait_ready()
# system config area (flash cells, but affect operation): table 12
@@ -230,6 +231,7 @@ class NFCHandler:
if done:
# do not wipe if we are not done
await self.wipe(kws.get("is_secret", False))
+ break
async def share_signed_txn(self, txid, file_offset, txn_len, txn_sha):
# we just signed something, share it over NFC
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.