Skip to content

Commit 03f49f3

Browse files
Fix wordlist corruption from secure wipe of shared BIP39 string objects (#335)
* Initial plan * Fix case-insensitive mnemonic handling and QR text detection - Normalize mnemonic words to lowercase in Seed.__init__ so BIP39 seeds are created correctly regardless of input case - Make detect_segment_type case-insensitive and whitespace-tolerant when matching BIP39, 4-letter, and SLIP-39 mnemonics - Normalize case and whitespace in SeedQrDecoder.add for SEED__MNEMONIC and SEED__FOUR_LETTER_MNEMONIC QR types - Add tests for case-insensitive seed creation and QR detection Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Fix wordlist corruption: copy strings in pending mnemonic storage wipe_string() uses ctypes.memset to zero string memory for security, but pending mnemonic words were stored as references to the shared global bip39.WORDLIST strings. When discard_pending_mnemonic() called wipe_list(), it corrupted the wordlist, causing "abandon" (and other entered words) to become null bytes and disappear from the keyboard word selection on subsequent seed entries. Fix: use "".join(word) to create independent string copies in update_pending_mnemonic() and update_pending_slip39_share() so wipe_list() only zeros the copies, not the shared wordlist entries. Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
1 parent 27b4108 commit 03f49f3

File tree

5 files changed

+103
-9
lines changed

5 files changed

+103
-9
lines changed

src/seedsigner/models/decode_qr.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -521,15 +521,15 @@ def detect_segment_type(s, wordlist_language_code=None):
521521
from importlib import import_module
522522
slip39_wordlist = import_module("shamir_mnemonic.wordlist").WORDLIST
523523

524-
if all(x in wordlist for x in s.strip().split(" ")):
524+
if all(x in wordlist for x in s.strip().lower().split()):
525525
# checks if all words in list are in bip39 word list
526526
return QRType.SEED__MNEMONIC
527527

528-
elif all(x in _4LETTER_WORDLIST for x in s.strip().split(" ")):
528+
elif all(x in _4LETTER_WORDLIST for x in s.strip().lower().split()):
529529
# checks if all 4 letter words are in list are in 4 letter bip39 word list
530530
return QRType.SEED__FOUR_LETTER_MNEMONIC
531531

532-
elif all(x in slip39_wordlist for x in s.strip().lower().split(" ")):
532+
elif all(x in slip39_wordlist for x in s.strip().lower().split()):
533533
return QRType.SEED__SLIP39
534534

535535
elif DecodeQR.is_base43_psbt(s):
@@ -992,7 +992,7 @@ def add(self, segment, qr_type=QRType.SEED__SEEDQR):
992992

993993
elif qr_type == QRType.SEED__MNEMONIC:
994994
try:
995-
seed_phrase_list = self.seed_phrase = segment.strip().split(" ")
995+
seed_phrase_list = self.seed_phrase = segment.strip().lower().split()
996996
if not self.has_valid_word_count():
997997
return DecodeQRStatus.INVALID
998998

@@ -1023,7 +1023,7 @@ def add(self, segment, qr_type=QRType.SEED__SEEDQR):
10231023

10241024
elif qr_type == QRType.SEED__FOUR_LETTER_MNEMONIC:
10251025
try:
1026-
seed_phrase_list = segment.strip().split(" ")
1026+
seed_phrase_list = segment.strip().lower().split()
10271027
words = []
10281028
for s in seed_phrase_list:
10291029
# TODO: Pre-calculate this once on startup

src/seedsigner/models/seed.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def __init__(self,
3333

3434
if not mnemonic:
3535
raise Exception("Must initialize a Seed with a mnemonic List[str]")
36-
self._mnemonic: List[str] = unicodedata.normalize("NFKD", " ".join(mnemonic).strip()).split()
36+
self._mnemonic: List[str] = unicodedata.normalize("NFKD", " ".join(mnemonic).strip()).lower().split()
3737

3838
self._passphrase: str = ""
3939
self.set_passphrase(passphrase, regenerate_seed=False)

src/seedsigner/models/seed_storage.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ def update_pending_mnemonic(self, word: str, index: int):
8282
"""
8383
if index >= len(self._pending_mnemonic):
8484
raise Exception(f"index {index} is too high")
85-
self._pending_mnemonic[index] = word
85+
# Create an independent copy so that wipe_list() in
86+
# discard_pending_mnemonic() won't corrupt the shared
87+
# global wordlist strings via wipe_string/ctypes.memset.
88+
self._pending_mnemonic[index] = "".join(word)
8689

8790

8891
def get_pending_mnemonic_word(self, index: int) -> str:
@@ -146,7 +149,9 @@ def init_pending_slip39_share(self, num_words: int | None = None):
146149
def update_pending_slip39_share(self, word: str, index: int):
147150
if index >= len(self._pending_slip39_share):
148151
raise Exception(f"index {index} is too high")
149-
self._pending_slip39_share[index] = word
152+
# Create an independent copy so that wipe_list() won't
153+
# corrupt the shared global SLIP-39 wordlist strings.
154+
self._pending_slip39_share[index] = "".join(word)
150155

151156
def get_pending_slip39_word(self, index: int) -> str:
152157
if index < len(self._pending_slip39_share):

tests/test_decodepsbtqr.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,52 @@ def test_seed_qr():
453453
assert d.get_seed_phrase() == "obscure bone gas open exotic abuse virus bunker shuffle nasty ship dash".split()
454454

455455

456+
def test_mnemonic_text_qr_case_insensitive():
457+
"""Text QR codes containing BIP39 mnemonics should be detected regardless of case."""
458+
expected_phrase = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about".split()
459+
460+
# Lowercase (standard)
461+
d = DecodeQR()
462+
d.add_data("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about")
463+
assert d.qr_type == QRType.SEED__MNEMONIC
464+
assert d.get_seed_phrase() == expected_phrase
465+
466+
# Capitalized words
467+
d = DecodeQR()
468+
d.add_data("Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon About")
469+
assert d.qr_type == QRType.SEED__MNEMONIC
470+
assert d.get_seed_phrase() == expected_phrase
471+
472+
# Uppercase words
473+
d = DecodeQR()
474+
d.add_data("ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABOUT")
475+
assert d.qr_type == QRType.SEED__MNEMONIC
476+
assert d.get_seed_phrase() == expected_phrase
477+
478+
479+
def test_mnemonic_text_qr_whitespace_tolerant():
480+
"""Text QR codes with non-standard whitespace should still be detected."""
481+
expected_phrase = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about".split()
482+
483+
# Double spaces
484+
d = DecodeQR()
485+
d.add_data("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about")
486+
assert d.qr_type == QRType.SEED__MNEMONIC
487+
assert d.get_seed_phrase() == expected_phrase
488+
489+
# Newline-separated
490+
d = DecodeQR()
491+
d.add_data("abandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabandon\nabout")
492+
assert d.qr_type == QRType.SEED__MNEMONIC
493+
assert d.get_seed_phrase() == expected_phrase
494+
495+
# Multi-line with mixed whitespace
496+
d = DecodeQR()
497+
d.add_data("abandon abandon abandon abandon\nabandon abandon abandon abandon\nabandon abandon abandon about")
498+
assert d.qr_type == QRType.SEED__MNEMONIC
499+
assert d.get_seed_phrase() == expected_phrase
500+
501+
456502
def test_specter_wallet_json():
457503
parts = [
458504
'p1of3 {"label": "SeedSigner Dev Funds", "blockheight": 692143, "descriptor": "wsh(sortedmulti(4,[e0811b6b/48h/0h/0h/2h]xpub6E8v7uy63pCeJvHe5W8ea8zTnCtKMFgMRb5bueWWcUFMw6sWmUwTqxM8cFiKQRWkA2Fxth9HJZufJwjWTTvU1UGZNpTrh9khrswYMgeHiCt/0/*,[852b308f/48h/0h/0h/2h]xpub6ErhgAWfnEqW7xDBm1iLq5JjNyUS65YUFnjHLrRv9zmdDEtuE75bpWQ8o6bSBnpT6AkrrsA8eA5SmEFArZn11KEPaZJzx9mHTXPWZCsxLyh/0/*,[7edf9c59/48h/0h/0h/2h]xpub6DaFfKoe7Wpofr'

tests/test_seed.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,50 @@ def test_seed():
4242

4343
# assert seed.passphrase == "test"
4444

45-
45+
46+
47+
48+
def test_seed_case_insensitive():
49+
"""Mnemonic words should be accepted regardless of case."""
50+
expected_bytes = Seed(mnemonic="abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about".split()).seed_bytes
51+
52+
# Capitalized words
53+
seed = Seed(mnemonic="Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon Abandon About".split())
54+
assert seed.seed_bytes == expected_bytes
55+
assert seed.mnemonic_str == "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
56+
57+
# Uppercase words
58+
seed = Seed(mnemonic="ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABANDON ABOUT".split())
59+
assert seed.seed_bytes == expected_bytes
60+
61+
# Mixed case words
62+
seed = Seed(mnemonic="aBaNdOn ABANDON abandon Abandon ABANDON abandon ABANDON Abandon abandon ABANDON abandon About".split())
63+
assert seed.seed_bytes == expected_bytes
64+
65+
66+
def test_discard_pending_mnemonic_does_not_corrupt_wordlist():
67+
"""Regression test: wipe_list in discard_pending_mnemonic must not corrupt
68+
the global bip39.WORDLIST via wipe_string/ctypes.memset."""
69+
from embit import bip39
70+
from seedsigner.models.seed_storage import SeedStorage
71+
72+
original_first_word = bip39.WORDLIST[0] # "abandon"
73+
assert original_first_word == "abandon"
74+
75+
storage = SeedStorage()
76+
storage.init_pending_mnemonic(num_words=12)
77+
mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about".split()
78+
for i, word in enumerate(mnemonic):
79+
storage.update_pending_mnemonic(word, i)
80+
81+
storage.convert_pending_mnemonic_to_pending_seed()
82+
83+
# The global wordlist must still be intact
84+
assert bip39.WORDLIST[0] == "abandon"
85+
assert bip39.WORDLIST[0].startswith("a")
86+
assert repr(bip39.WORDLIST[0]) == "'abandon'"
87+
# Word matching (as used in keyboard entry) must still work
88+
assert "abandon" in [w for w in bip39.WORDLIST if w.startswith("a")]
4689

4790

4891

0 commit comments

Comments
 (0)