fix: return None when decrypting an unknown or non-dict mnemonic id
What changed, and why it matters
This commit fixes a small bug in how Krux loads saved encrypted seed data. Previously, if the saved file was valid JSON but shaped like a list instead of a dictionary, or if the requested seed ID was missing, the code could crash with an AttributeError instead of cleanly returning None. The fix makes the code treat those cases as 'nothing found' and adds tests to confirm it. The commit message says the on-device behavior is unchanged because the only caller already catches exceptions.
Treat as a routine defensive fix. Review whether any other callers of decrypt() or storage loaders exist outside the patched module and ensure they also handle None returns. No urgent security response is indicated by the commit materials.
Security signals we found
Fixes exception-handling bug that could crash decrypt() on missing or malformed storage entries
Adds input validation for JSON shape (dict vs list/other) when loading mnemonic storage
Adds unit tests covering malformed/non-dict JSON and unknown mnemonic IDs
Removes bare except in decrypt() lookup path
Evidence from the diff
MnemonicStorage.decrypt() previously wrapped only the dict.get() lookup in a try/except, then accessed stored_value.get(‘b64_kef’) unconditionally. If the id was unknown, stored_value was None and the subsequent .get() raised AttributeError. The same happened when seeds.json parsed to a non-dict (e.g., a JSON array). The patch introduces _load_mnemonics() to coerce non-dict JSON to {} during init and store_encrypted_kef(), and changes decrypt() to return None when the storage source or stored entry is not a dict. Tests are added for non-dict JSON loads and for decrypt() with unknown ids/non-dict storage.
Changed components
src/krux/encryption.pytests/test_encryption.pyMnemonicStorage classMnemonicStorage.decrypt()MnemonicStorage._load_mnemonics()MnemonicStorage.store_encrypted_kef()Inspect captured patch +101 / −10
diff --git a/src/krux/encryption.py b/src/krux/encryption.py
index f669937..4bca6ca 100644
--- a/src/krux/encryption.py
+++ b/src/krux/encryption.py
@@ -36,18 +36,23 @@ QR_CODE_ITER_MULTIPLE = 10000
class MnemonicStorage:
"""Handler of stored encrypted seeds"""
+ @staticmethod
+ def _load_mnemonics(contents):
+ mnemonics = json.loads(contents)
+ return mnemonics if isinstance(mnemonics, dict) else {}
+
def __init__(self) -> None:
self.stored = {}
self.stored_sd = {}
try:
with SDHandler() as sd:
- self.stored_sd = json.loads(sd.read(MNEMONICS_FILE))
+ self.stored_sd = self._load_mnemonics(sd.read(MNEMONICS_FILE))
except (OSError, ValueError):
# missing/unreadable SD card or malformed JSON -> start empty
pass
try:
with open(FLASH_PATH_STR % MNEMONICS_FILE, "r") as f:
- self.stored = json.loads(f.read())
+ self.stored = self._load_mnemonics(f.read())
except (OSError, ValueError):
# missing/unreadable flash file or malformed JSON -> start empty
pass
@@ -89,12 +94,10 @@ class MnemonicStorage:
def decrypt(self, key, mnemonic_id, sd_card=False):
"""Decrypt a selected encrypted mnemonic from a file"""
- try:
- if sd_card:
- stored_value = self.stored_sd.get(mnemonic_id)
- else:
- stored_value = self.stored.get(mnemonic_id)
- except:
+ source = self.stored_sd if sd_card else self.stored
+ stored_value = source.get(mnemonic_id) if isinstance(source, dict) else None
+ if not isinstance(stored_value, dict):
+ # unknown id, or a corrupt/non-dict storage entry -> nothing to decrypt
return None
if stored_value.get("b64_kef"):
@@ -122,7 +125,7 @@ class MnemonicStorage:
with SDHandler() as sd:
contents = sd.read(MNEMONICS_FILE)
orig_len = len(contents)
- mnemonics = json.loads(contents)
+ mnemonics = self._load_mnemonics(contents)
except (OSError, ValueError):
# no existing/readable file or malformed JSON -> write fresh
orig_len = 0
@@ -143,7 +146,7 @@ class MnemonicStorage:
try:
# load current MNEMONICS_FILE
with open(FLASH_PATH_STR % MNEMONICS_FILE, "r") as f:
- mnemonics = json.loads(f.read())
+ mnemonics = self._load_mnemonics(f.read())
except (OSError, ValueError):
# no existing/readable file or malformed JSON -> write fresh
pass
diff --git a/tests/test_encryption.py b/tests/test_encryption.py
index ef430a8..22f9f30 100644
--- a/tests/test_encryption.py
+++ b/tests/test_encryption.py
@@ -661,6 +661,19 @@ def test_init_sd_load_malformed_json_starts_empty(m5stickv, mocker):
assert storage.stored_sd == {}
+def test_init_sd_load_non_dict_json_starts_empty(m5stickv, mocker):
+ from krux.encryption import MnemonicStorage
+
+ sd = mocker.MagicMock()
+ sd.read.return_value = "[1, 2, 3]"
+ sdhandler = mocker.MagicMock()
+ sdhandler.return_value.__enter__.return_value = sd
+ mocker.patch("krux.encryption.SDHandler", new=sdhandler)
+ with patch("krux.encryption.open", new=mocker.mock_open(read_data="{}")):
+ storage = MnemonicStorage()
+ assert storage.stored_sd == {}
+
+
# --- __init__ flash load (self.stored) ---
@@ -694,6 +707,16 @@ def test_init_flash_load_malformed_json_starts_empty(m5stickv, mocker):
assert storage.stored == {}
+def test_init_flash_load_non_dict_json_starts_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 == {}
+ assert storage.list_mnemonics() == []
+
+
# --- store_encrypted_kef SD read-before-write ---
@@ -743,6 +766,23 @@ def test_store_sd_read_malformed_json_still_writes(
m().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
+def test_store_sd_read_non_dict_json_still_writes(
+ m5stickv, mocker, mock_file_operations
+):
+ from krux.krux_settings import Settings
+ from krux.encryption import MnemonicStorage
+
+ 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:
+ 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)
+
+
# --- store_encrypted_kef flash read-before-write ---
@@ -795,3 +835,51 @@ def test_store_flash_read_malformed_json_still_writes(m5stickv, mocker):
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)
+
+
+def test_store_flash_read_non_dict_json_still_writes(m5stickv, mocker):
+ from krux.krux_settings import Settings
+ from krux.encryption import MnemonicStorage
+
+ 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()
+ 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)
+
+
+# ---------------------------------------------------------------------------
+# decrypt() must not crash on a missing id or non-dict storage.
+#
+# storage.get(id) returns None for an unknown id. decrypt() should return None
+# instead of raising AttributeError when there is no stored entry.
+# ---------------------------------------------------------------------------
+
+
+def test_decrypt_unknown_id_returns_none(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="{}")):
+ storage = MnemonicStorage()
+ # both stores are empty; an unknown id must return None, not crash
+ assert storage.decrypt("any-key", "no-such-id", sd_card=False) is None
+ assert storage.decrypt("any-key", "no-such-id", sd_card=True) is None
+
+
+def test_decrypt_non_dict_storage_returns_none(m5stickv, mocker):
+ from krux.encryption import MnemonicStorage
+
+ mocker.patch("krux.encryption.SDHandler", side_effect=OSError)
+ # valid JSON that is not an object -> storage starts empty
+ with patch("krux.encryption.open", new=mocker.mock_open(read_data="[1, 2, 3]")):
+ storage = MnemonicStorage()
+ assert storage.stored == {}
+ assert storage.decrypt("any-key", "any-id", sd_card=False) is None
Why this scored 29/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.