fix: preserve corrupt seeds.json instead of overwriting on store
What changed, and why it matters
This commit fixes a bug in Krux, a Bitcoin hardware-wallet tool, where saving a new encrypted seed could silently overwrite a damaged seeds.json file. The patch makes the app detect corrupt or misshapen seed storage and stop the save, preserving the existing file so the user can recover it. It also prevents the seed list menu from crashing when the stored file is malformed. There is no evidence this was a malicious backdoor; it appears to be a defensive bug fix.
Review as a normal defensive fix. No urgent security response required. Verify that the English-only error string is acceptable for the project's localization policy, and consider whether users need documentation on how to recover a preserved corrupt seeds.json.
Security signals we found
Data-loss prevention: stops silent overwrite of recoverable seed storage
Defensive input validation: rejects non-dict JSON shape before write
Exception-based guard added to storage write path
UI now surfaces corruption instead of silently proceeding
No authentication, crypto, or access-control changes observed
Evidence from the diff
The change introduces StorageCorruptedError and updates MnemonicStorage.store_encrypted_kef() to raise it instead of overwriting when an existing seeds.json is malformed JSON or a non-dict object, for both flash and SD-card paths. list_mnemonics() now returns [] for non-dict storage. The UI catches the exception and shows an English-only error, aborting the store. Tests verify the file is not opened for write/truncation when corruption is detected.
Changed components
src/krux/encryption.pysrc/krux/pages/encryption_ui.pytests/test_encryption.pytests/pages/test_encryption_ui.pyInspect captured patch +167 / −25
diff --git a/src/krux/encryption.py b/src/krux/encryption.py
index f2520e9..8cacf64 100644
--- a/src/krux/encryption.py
+++ b/src/krux/encryption.py
@@ -33,6 +33,11 @@ FLASH_PATH_STR = "/" + FLASH_PATH + "/%s"
QR_CODE_ITER_MULTIPLE = 10000
+class StorageCorruptedError(Exception):
+ """Stored mnemonics file exists but is not a valid object; it is left
+ untouched instead of being overwritten, so its data can be recovered."""
+
+
class MnemonicStorage:
"""Handler of stored encrypted seeds"""
@@ -85,11 +90,11 @@ class MnemonicStorage:
def list_mnemonics(self, sd_card=False):
"""List all seeds stored on a file"""
- mnemonic_ids = []
source = self.stored_sd if sd_card else self.stored
- for mnemonic_id in source:
- mnemonic_ids.append(mnemonic_id)
- return mnemonic_ids
+ if not isinstance(source, dict):
+ # corrupt/non-dict storage -> nothing to list
+ return []
+ return list(source)
def decrypt(self, key, mnemonic_id, sd_card=False):
"""Decrypt a selected encrypted mnemonic from a file"""
@@ -120,14 +125,21 @@ class MnemonicStorage:
mnemonics = {}
if sd_card:
# load current MNEMONICS_FILE
+ orig_len = 0
try:
with SDHandler() as sd:
contents = sd.read(MNEMONICS_FILE)
orig_len = len(contents)
mnemonics = self._load_mnemonics(contents)
- except (OSError, ValueError):
- # no existing/readable file or malformed JSON -> write fresh
- orig_len = 0
+ except OSError:
+ # missing file -> write a fresh store
+ pass
+ except ValueError as exc:
+ # corrupt JSON -> preserve for recovery
+ raise StorageCorruptedError(MNEMONICS_FILE) from exc
+ if not isinstance(mnemonics, dict):
+ # wrong shape -> preserve for recovery
+ raise StorageCorruptedError(MNEMONICS_FILE)
# save the new MNEMONICS_FILE
try:
@@ -146,9 +158,15 @@ class MnemonicStorage:
# load current MNEMONICS_FILE
with open(FLASH_PATH_STR % MNEMONICS_FILE, "r") as f:
mnemonics = self._load_mnemonics(f.read())
- except (OSError, ValueError):
- # no existing/readable file or malformed JSON -> write fresh
+ except OSError:
+ # missing file -> write a fresh store
pass
+ except ValueError as exc:
+ # corrupt JSON -> preserve for recovery
+ raise StorageCorruptedError(MNEMONICS_FILE) from exc
+ if not isinstance(mnemonics, dict):
+ # wrong shape -> preserve for recovery
+ raise StorageCorruptedError(MNEMONICS_FILE)
try:
# save the new MNEMONICS_FILE
with open(FLASH_PATH_STR % MNEMONICS_FILE, "w") as f:
diff --git a/src/krux/pages/encryption_ui.py b/src/krux/pages/encryption_ui.py
index 11f9fc3..25cca9d 100644
--- a/src/krux/pages/encryption_ui.py
+++ b/src/krux/pages/encryption_ui.py
@@ -615,7 +615,7 @@ class EncryptMnemonic(Page):
def store_mnemonic_on_memory(self, sd_card=False):
"""Save encrypted mnemonic on flash or sd_card"""
- from ..encryption import MnemonicStorage
+ from ..encryption import MnemonicStorage, StorageCorruptedError
encrypted_data, mnemonic_id = self._encrypt_mnemonic_with_label()
if encrypted_data is None:
@@ -629,7 +629,23 @@ class EncryptMnemonic(Page):
del mnemonic_storage
return
- if mnemonic_storage.store_encrypted_kef(mnemonic_id, encrypted_data, sd_card):
+ try:
+ stored = mnemonic_storage.store_encrypted_kef(
+ mnemonic_id, encrypted_data, sd_card
+ )
+ except StorageCorruptedError:
+ self.ctx.display.clear()
+ # English-only: rare corruption guard, not worth translating
+ self.ctx.display.draw_centered_text(
+ "Stored seeds file is corrupted and was preserved.\n"
+ "Encrypted mnemonic was not stored.",
+ theme.error_color,
+ )
+ self.ctx.input.wait_for_button()
+ del mnemonic_storage
+ return
+
+ if stored:
self.ctx.display.clear()
self.ctx.display.draw_centered_text(
t("Encrypted mnemonic stored with ID:") + " " + mnemonic_id,
diff --git a/tests/pages/test_encryption_ui.py b/tests/pages/test_encryption_ui.py
index 5a65179..b6749ef 100644
--- a/tests/pages/test_encryption_ui.py
+++ b/tests/pages/test_encryption_ui.py
@@ -41,7 +41,7 @@ def mock_file_operations(mocker):
"os.listdir",
new=mocker.MagicMock(return_value=["somefile", "otherfile"]),
)
- mocker.patch("builtins.open", mocker.mock_open(read_data="SEEDS_JSON"))
+ mocker.patch("builtins.open", mocker.mock_open(read_data=SEEDS_JSON))
def test_load_key_from_keypad(m5stickv, mocker):
@@ -289,6 +289,49 @@ def test_encrypt_save_error(m5stickv, mocker, mock_file_operations):
assert ctx.input.wait_for_button.call_count == len(BTN_SEQUENCE)
+def test_encrypt_save_corrupted_file_preserved(m5stickv, mocker, mock_file_operations):
+ from krux.wallet import Wallet
+ from krux.krux_settings import Settings
+ from krux.input import BUTTON_ENTER
+ from krux.pages.encryption_ui import EncryptMnemonic
+ from krux.encryption import StorageCorruptedError
+ from krux.key import Key
+ from embit.networks import NETWORKS
+ from krux.themes import theme
+
+ BTN_SEQUENCE = (
+ [BUTTON_ENTER] # Confirm flash store
+ + [BUTTON_ENTER] # Yes, use fingerprint as ID
+ + [BUTTON_ENTER] # Confirm encryption ID
+ )
+ ctx = create_ctx(mocker, BTN_SEQUENCE)
+ ctx.wallet = Wallet(Key(ECB_WORDS, False, NETWORKS["main"]))
+ Settings().encryption.version = "AES-ECB"
+ storage_ui = EncryptMnemonic(ctx)
+ mocker.patch(
+ "krux.pages.encryption_ui.EncryptionKey.encryption_key",
+ mocker.MagicMock(return_value=TEST_KEY),
+ )
+ # a corrupt seeds file must not be overwritten: store raises, UI warns
+ mocker.patch(
+ "krux.encryption.MnemonicStorage.store_encrypted_kef",
+ mocker.MagicMock(side_effect=StorageCorruptedError("seeds.json")),
+ )
+ storage_ui.encrypt_menu()
+
+ ctx.display.draw_centered_text.assert_has_calls(
+ [
+ mocker.call(
+ "Stored seeds file is corrupted and was preserved.\n"
+ "Encrypted mnemonic was not stored.",
+ theme.error_color,
+ )
+ ],
+ any_order=True,
+ )
+ assert ctx.input.wait_for_button.call_count == len(BTN_SEQUENCE)
+
+
def test_encrypt_to_qrcode_ecb_ui(m5stickv, mocker):
from krux.wallet import Wallet
from krux.krux_settings import Settings
diff --git a/tests/test_encryption.py b/tests/test_encryption.py
index fafd1b4..a6d8e3f 100644
--- a/tests/test_encryption.py
+++ b/tests/test_encryption.py
@@ -726,21 +726,35 @@ def test_store_sd_read_oserror_still_writes(m5stickv, mocker, mock_file_operatio
m().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-def test_store_sd_read_malformed_json_still_writes(
+def test_store_sd_read_malformed_json_raises_and_preserves(
m5stickv, mocker, mock_file_operations
):
from krux.krux_settings import Settings
- from krux.encryption import MnemonicStorage
+ from krux.encryption import MnemonicStorage, StorageCorruptedError
storage = MnemonicStorage()
Settings().encryption.version = "AES-ECB"
mocker.patch("krux.sd_card.SDHandler.read", return_value="not valid json {{{")
with patch("krux.sd_card.open", new=mocker.mock_open(read_data="{}")) as m:
- success = storage.store_encrypted_kef(
- "KEFecbID", KEF_ENVELOPE_ECB, sd_card=True
- )
- assert success is True
- m().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
+ with pytest.raises(StorageCorruptedError):
+ storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=True)
+ # existing (corrupt-but-recoverable) file must not be overwritten
+ m().write.assert_not_called()
+
+
+def test_store_sd_read_non_dict_json_raises_and_preserves(
+ m5stickv, mocker, mock_file_operations
+):
+ from krux.krux_settings import Settings
+ from krux.encryption import MnemonicStorage, StorageCorruptedError
+
+ storage = MnemonicStorage()
+ Settings().encryption.version = "AES-ECB"
+ mocker.patch("krux.sd_card.SDHandler.read", return_value="[1, 2, 3]")
+ with patch("krux.sd_card.open", new=mocker.mock_open(read_data="{}")) as m:
+ with pytest.raises(StorageCorruptedError):
+ storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=True)
+ m().write.assert_not_called()
# --- store_encrypted_kef flash read-before-write ---
@@ -779,22 +793,24 @@ def test_store_flash_read_oserror_still_writes(m5stickv, mocker):
write_handle().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-def test_store_flash_read_malformed_json_still_writes(m5stickv, mocker):
+def test_store_flash_read_malformed_json_raises_and_preserves(m5stickv, mocker):
from krux.krux_settings import Settings
- from krux.encryption import MnemonicStorage
+ from krux.encryption import MnemonicStorage, StorageCorruptedError
with patch("krux.encryption.open", new=mocker.mock_open(read_data="{}")):
storage = MnemonicStorage()
Settings().encryption.version = "AES-ECB"
read_handle = mocker.mock_open(read_data="not valid json {{{")
write_handle = mocker.mock_open()
- mocker.patch(
+ open_mock = mocker.patch(
"krux.encryption.open",
side_effect=[read_handle.return_value, write_handle.return_value],
)
- success = storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=False)
- assert success is True
- write_handle().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
+ with pytest.raises(StorageCorruptedError):
+ storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=False)
+ # file opened for read only; never opened for write (no truncation)
+ assert open_mock.call_count == 1
+ write_handle().write.assert_not_called()
# ---------------------------------------------------------------------------
@@ -825,3 +841,52 @@ def test_decrypt_non_dict_storage_returns_none(m5stickv, mocker):
storage = MnemonicStorage()
assert storage.stored == [1, 2, 3]
assert storage.decrypt("any-key", "any-id", sd_card=False) is None
+
+
+# list_mnemonics() returns [] for non-dict storage instead of iterating it
+# (a list would yield junk ids; a non-iterable like null would raise).
+
+
+def test_list_mnemonics_non_dict_storage_returns_empty(m5stickv, mocker):
+ from krux.encryption import MnemonicStorage
+
+ mocker.patch("krux.encryption.SDHandler", side_effect=OSError)
+ with patch("krux.encryption.open", new=mocker.mock_open(read_data="[1, 2, 3]")):
+ storage = MnemonicStorage()
+ assert storage.stored == [1, 2, 3]
+ assert storage.list_mnemonics(sd_card=False) == []
+
+
+def test_list_mnemonics_non_iterable_storage_returns_empty(m5stickv, mocker):
+ from krux.encryption import MnemonicStorage
+
+ mocker.patch("krux.encryption.SDHandler", side_effect=OSError)
+ # JSON "null" loads to None, which is not iterable -> must not raise
+ with patch("krux.encryption.open", new=mocker.mock_open(read_data="null")):
+ storage = MnemonicStorage()
+ assert storage.stored is None
+ assert storage.list_mnemonics(sd_card=False) == []
+
+
+# store_encrypted_kef() raises before opening "w" on a non-dict flash file,
+# so the existing (recoverable) data is never truncated.
+
+
+def test_store_flash_read_non_dict_json_raises_and_preserves(m5stickv, mocker):
+ from krux.krux_settings import Settings
+ from krux.encryption import MnemonicStorage, StorageCorruptedError
+
+ with patch("krux.encryption.open", new=mocker.mock_open(read_data="{}")):
+ storage = MnemonicStorage()
+ Settings().encryption.version = "AES-ECB"
+ read_handle = mocker.mock_open(read_data="[1, 2, 3]")
+ write_handle = mocker.mock_open()
+ open_mock = mocker.patch(
+ "krux.encryption.open",
+ side_effect=[read_handle.return_value, write_handle.return_value],
+ )
+ with pytest.raises(StorageCorruptedError):
+ storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=False)
+ # file opened for read only; never opened for write (no truncation)
+ assert open_mock.call_count == 1
+ write_handle().write.assert_not_called()
Why this scored 54/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.