What changed, and why it matters
This commit fixes the 'first and last seed word' login challenge on older COLDCARD Mk4 devices. Previously, the on-screen word picker for this security check was broken (the code even had a 'TODO: fix bugs here on Mk4' comment), which could have allowed someone with physical access to bypass or fail the word-check screen. The patch replaces the buggy callback-based picker with a working one, so the device can now properly verify the user's seed words during login.
Treat this as a security-relevant bug fix and include it in the next firmware release. Users on affected Mk4 firmware should upgrade to a version containing this commit. Review whether the broken challenge window could have been exploited to bypass login or trick users into accepting a different seed, and consider a security note or advisory if further analysis confirms a bypass.
Security signals we found
Fixes a broken authentication/verification UI flow (Mk4 SSSP word challenge)
Removes a developer TODO comment indicating known bugs in the previous implementation
Adds explicit guard so word challenge only applies to BIP39 seed mode
Replaces closure-scoped variable capture with class-level state to ensure chosen words are returned
Potential security relevance: a non-functional challenge could weaken or confuse the login-time seed verification
Evidence from the diff
The change refactors sssp_word_challenge() in shared/ccc.py and adds a dedicated word-selection path in shared/seed.py. Before, on non-QWERTY devices (Mk4), WordNestMenu was instantiated with a done_cb that set a local got_words variable inside a nested async function; because that variable was never propagated out, the challenge could not complete correctly. The patch moves the logic into WordNestMenu.login_sequence_word_check() and menu_done_cbf(), which collect the two chosen words into the class-level words list and return them. It also tightens the SensitiveValues guard so the word challenge only runs when the seed is actually BIP39 words. The net effect is that the Mk4 SSSP word challenge now functions as intended.
Changed components
shared/ccc.pyshared/seed.pyMk4 SSSP (Seed Secret Security Protocol) word challengeWordNestMenu UI componentInspect captured patch +37 / −20
diff --git a/shared/ccc.py b/shared/ccc.py
index 546ff8f..cf9f412 100644
--- a/shared/ccc.py
+++ b/shared/ccc.py
@@ -1121,15 +1121,15 @@ async def sssp_word_challenge(*a):
from stash import SensitiveValues
with SensitiveValues() as sv:
- if sv.mode == 'words':
- words = bip39.b2a_words(sv.raw).split(' ')
- want_words = words[:1] + words[-1:]
- assert len(want_words) == 2
- else:
+ if sv.mode != 'words':
# they are using XPRV or something, skip test entirely
return
- got_words = None
+ words = bip39.b2a_words(sv.raw).split(' ')
+ want_words = words[:1] + words[-1:]
+ assert len(want_words) == 2
+
+ got_words = []
for retry in range(2):
if version.has_qwerty:
# see special rendering code for this case in ux_q1.py:ux_draw_words(num_words=2)
@@ -1137,23 +1137,14 @@ async def sssp_word_challenge(*a):
got_words = await seed_word_entry('First and Last Seed Words', 2, has_checksum=False)
else:
from seed import WordNestMenu
-
- # TODO: fix bugs here on Mk4. really not working. XXX
-
- got_words = None
- async def check_challenge_cb(words):
- WordNestMenu.pop_all()
- got_words = words
-
- m = WordNestMenu(num_words=2, has_checksum=False, done_cb=check_challenge_cb)
- the_ux.push(m)
- await m.interact()
+ got_words = await WordNestMenu.login_sequence_word_check()
if got_words == want_words:
# success - done
return
await ux_show_story("Sorry, those words are incorrect.")
+ got_words = []
# they failed; log them out ... they can just try login again
from actions import login_now
diff --git a/shared/seed.py b/shared/seed.py
index 428aa2f..26f6181 100644
--- a/shared/seed.py
+++ b/shared/seed.py
@@ -150,23 +150,49 @@ class WordNestMenu(MenuSystem):
done_cb = None
def __init__(self, num_words=None, has_checksum=True, done_cb=commit_new_words,
- items=None, is_commit=False):
+ items=None, is_commit=False, menu_cbf=None, prefix=""):
if num_words is not None:
WordNestMenu.target_words = num_words
WordNestMenu.has_checksum = has_checksum
WordNestMenu.words = []
- assert done_cb
WordNestMenu.done_cb = done_cb
is_commit = True
if not items:
- items = [MenuItem(i, menu=self.next_menu) for i in letter_choices()]
+ ch = letter_choices(prefix)
+ if menu_cbf:
+ items = [MenuItem(i, f=menu_cbf) for i in ch]
+ else:
+ items = [MenuItem(i, menu=self.next_menu) for i in ch]
self.is_commit = is_commit
super(WordNestMenu, self).__init__(items)
+ @classmethod
+ async def menu_done_cbf(cls, a, b, c):
+ if c.label[-1] == '-':
+ lc = c.label[0:-1]
+ else:
+ lc = ""
+ cls.words.append(c.label)
+ if len(cls.words) >= 2:
+ from glob import numpad
+ numpad.abort_ux()
+ return
+
+ m = cls(prefix=lc, menu_cbf=cls.menu_done_cbf)
+ the_ux.push(m)
+ await m.interact()
+
+ @classmethod
+ async def login_sequence_word_check(cls):
+ m = cls(num_words=2, menu_cbf=WordNestMenu.menu_done_cbf)
+ the_ux.push(m)
+ await the_ux.interact()
+ return cls.words
+
@staticmethod
async def next_menu(self, idx, choice):
Why this scored 44/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.