decouple wiping NFC chip from `ux_animation` routine
What the developer wrote
Authored by scgbckbone
decouple wiping NFC chip from `ux_animation` routine
What changed, and why it matters
This change moves the cleanup step that wipes sensitive data from the NFC chip so it happens when sharing is complete, rather than inside a lower-level animation routine. The old code only wiped the chip when the user aborted the NFC operation, which could mean secret data sat on the NFC chip longer than intended after a normal, successful share. The patch makes wiping happen reliably at the end of a successful share, but removes the wipe that used to occur on abort, so the security behavior depends on whether abort paths now clean up elsewhere.
Review the full NFC sharing flow to confirm that aborted sessions still wipe the chip, either in the caller or in a finally/exception handler. If the abort path no longer wipes, add an explicit wipe there. Otherwise, this change is likely a benign refactor.
Security signals we found
NFC secret data lifecycle changed: wipe moved from abort path to success path
Potential secret remnant on NFC chip if aborted path no longer wipes
No explicit security framing in commit message or diff
Partial view of control flow: abort cleanup behavior not shown in this diff
Evidence from the diff
The commit decouples NFC chip wiping from ux_animation. Previously, share_start/ux_animation called self.wipe(is_secret) only when not in write_mode and aborted was true. After the change, share_loop calls self.wipe(kws.get(‘is_secret’, False)) when share_start returns done (successful completion). The wipe is no longer performed inside ux_animation on abort. This shifts the security invariant from ‘wipe on abort’ to ‘wipe on successful done’, with no visible wipe on abort in this diff. The change is small and the commit message frames it as a decoupling/cleanup refactor, not a security fix.
Changed components
shared/nfc.pyNFCHandler.share_loopNFCHandler.ux_animationNFCHandler.share_startInspect captured patch +3 / −7
diff --git a/shared/nfc.py b/shared/nfc.py
index aff644a..2a48c2c 100644
--- a/shared/nfc.py
+++ b/shared/nfc.py
@@ -227,7 +227,9 @@ class NFCHandler:
async def share_loop(self, n, **kws):
while 1:
done = await self.share_start(n, **kws)
- if done: break
+ if done:
+ # do not wipe if we are not done
+ await self.wipe(kws.get("is_secret", False))
async def share_signed_txn(self, txid, file_offset, txn_len, txn_sha):
# we just signed something, share it over NFC
@@ -471,10 +473,6 @@ class NFCHandler:
break
self.set_rf_disable(1)
- # do not wipe if we are not aborted
- if not write_mode and aborted:
- # function argument secret decides whether to do full wipe after writing to chip
- await self.wipe(is_secret)
return aborted
@@ -482,9 +480,7 @@ class NFCHandler:
# do the UX while we are sharing a value over NFC
# - assumpting is people know what they are scanning
# - x key to abort early, but also self-clears
-
await self.big_write(ndef_obj.bytes())
-
return await self.ux_animation(False, **kws)
async def start_nfc_rx(self, **kws):
Why this scored 41/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.