fix: narrow bare except blocks to stop hiding real errors (#877)
What changed, and why it matters
This commit tightens several 'catch-all' error handlers in the Krux firmware so they only catch the specific problems they intend to handle. Previously, these handlers would also swallow serious system-level signals such as KeyboardInterrupt and unexpected programming errors, which could hide bugs or prevent a user from cancelling an operation. The change is defensive hardening rather than a fix for a known active attack, but it removes a class of reliability and safety bugs.
Treat as a worthwhile hardening change. Review whether any other bare `except:` blocks remain in the codebase and narrow them similarly. No immediate incident response is warranted unless an independent advisory links this pattern to an exploitable condition.
Security signals we found
Bare except blocks narrowed to specific exception types
KeyboardInterrupt and other BaseException signals no longer swallowed
Regression tests added to prevent silent error suppression
Defensive hardening in key derivation, QR display, QR parsing, and SD card detection
Evidence from the diff
The patch narrows bare except: clauses to except StopIteration, except IndexError, or except Exception in key.py, pages/__init__.py, and qr.py. Bare except: catches BaseException, including KeyboardInterrupt, SystemExit, and GeneratorExit. By narrowing the handlers, genuine control-flow signals and unexpected errors now propagate instead of being silently discarded. The accompanying tests verify that KeyboardInterrupt is no longer swallowed in extract_fingerprint, detect_format, and has_sd_card, and that real errors in display_qr_codes propagate rather than triggering a silent generator restart.
Changed components
src/krux/key.pysrc/krux/pages/__init__.pysrc/krux/qr.pyInspect captured patch +98 / −5
diff --git a/src/krux/key.py b/src/krux/key.py
index 7559f4e..8172e70 100644
--- a/src/krux/key.py
+++ b/src/krux/key.py
@@ -202,7 +202,7 @@ class Key:
Key.extract_root(mnemonic, passphrase, network).child(0).fingerprint,
pretty,
)
- except:
+ except Exception:
pass
return ""
diff --git a/src/krux/pages/__init__.py b/src/krux/pages/__init__.py
index ce97306..e04ef74 100644
--- a/src/krux/pages/__init__.py
+++ b/src/krux/pages/__init__.py
@@ -294,7 +294,7 @@ class Page:
while not done:
try:
code, num_parts = next(code_generator)
- except:
+ except StopIteration:
code_generator = to_qr_codes(data, qr_data_width, qr_format)
code, num_parts = next(code_generator)
@@ -523,7 +523,7 @@ class Page:
# Check for SD hot-plug
with SDHandler():
return True
- except:
+ except Exception:
return False
def shutdown(self):
diff --git a/src/krux/qr.py b/src/krux/qr.py
index 6345b37..29d7b04 100644
--- a/src/krux/qr.py
+++ b/src/krux/qr.py
@@ -308,7 +308,7 @@ def max_qr_bytes(max_width, encoding="byte"):
try:
return capacity_list[qr_version - 1]
- except:
+ except IndexError:
# Limited to version 20
return capacity_list[-1]
@@ -406,6 +406,6 @@ def detect_format(data):
bbqr_encoding = data[2]
return FORMAT_BBQR, BBQrCode(None, bbqr_encoding, bbqr_file_type)
- except:
+ except Exception:
pass
return qr_format, None
diff --git a/tests/pages/test_page.py b/tests/pages/test_page.py
index 3c4319a..b731697 100644
--- a/tests/pages/test_page.py
+++ b/tests/pages/test_page.py
@@ -137,6 +137,31 @@ def test_display_qr_code(mocker, m5stickv, mock_page_cls):
assert ctx.input.wait_for_button.call_count == len(BTN_SEQUENCE)
+def test_display_qr_code_propagates_real_errors(mocker, m5stickv, mock_page_cls):
+ """A non-StopIteration error from the QR generator must propagate and must
+ NOT trigger a silent generator restart.
+
+ Regression for narrowing the bare ``except`` to ``except StopIteration``.
+ With the old bare except, a real error was swallowed and ``to_qr_codes``
+ was called a second time; now it surfaces immediately.
+ """
+ from krux.qr import FORMAT_NONE
+
+ def boom(*args, **kwargs):
+ raise ValueError("bad qr data")
+ yield # pragma: no cover - makes boom a generator function
+
+ mocked = mocker.patch("krux.pages.to_qr_codes", side_effect=boom)
+ ctx = create_ctx(mocker, [])
+ page = mock_page_cls(ctx)
+
+ with pytest.raises(ValueError):
+ page.display_qr_codes(TEST_QR_DATA, FORMAT_NONE)
+
+ # The error surfaced on the first generator; no silent restart attempt.
+ assert mocked.call_count == 1
+
+
def test_display_qr_code_light_theme(mocker, m5stickv, mock_page_cls):
from krux.input import BUTTON_ENTER
from krux.qr import FORMAT_NONE
@@ -646,3 +671,24 @@ def test_fit_to_line_not_crop_middle(mocker, multiple_devices, mock_page_cls):
formatted_text = page.fit_to_line(case[TXT], case[PREFIX], crop_middle=False)
assert len(formatted_text) <= max_chars_in_line
assert formatted_text == case[device_type]
+
+
+def test_has_sd_card_handles_errors_and_propagates_signals(
+ mocker, m5stickv, mock_page_cls
+):
+ """has_sd_card returns False on genuine SD errors, but must NOT swallow
+ BaseException-level signals like KeyboardInterrupt.
+
+ Regression for narrowing the bare except to `except Exception`.
+ """
+ ctx = create_ctx(mocker, [])
+ page = mock_page_cls(ctx)
+
+ # Genuine SD failure (OSError) -> reported as "no SD card".
+ mocker.patch("krux.pages.SDHandler", side_effect=OSError("no card"))
+ assert page.has_sd_card() is False
+
+ # Shutdown signal during the check -> propagates, not swallowed into False.
+ mocker.patch("krux.pages.SDHandler", side_effect=KeyboardInterrupt)
+ with pytest.raises(KeyboardInterrupt):
+ page.has_sd_card()
diff --git a/tests/test_key.py b/tests/test_key.py
index cfc06b7..d4be6f3 100644
--- a/tests/test_key.py
+++ b/tests/test_key.py
@@ -728,3 +728,18 @@ def test_classmethod_extract_fingerprint(mocker, m5stickv, tdata):
fingerprint = Key.extract_fingerprint("this is not a mnemonic", pretty=False)
assert fingerprint == ""
+
+
+def test_extract_fingerprint_propagates_base_exceptions(mocker, m5stickv):
+ """extract_fingerprint catches genuine errors (returns "") but must NOT
+ swallow BaseException-level signals like KeyboardInterrupt.
+
+ Regression for narrowing the bare except to ``except Exception``.
+ """
+ import pytest
+ from krux.key import Key
+
+ mocker.patch.object(Key, "extract_root", side_effect=KeyboardInterrupt)
+
+ with pytest.raises(KeyboardInterrupt):
+ Key.extract_fingerprint("any mnemonic", pretty=False)
diff --git a/tests/test_qr.py b/tests/test_qr.py
index ccf524c..7572316 100644
--- a/tests/test_qr.py
+++ b/tests/test_qr.py
@@ -244,3 +244,35 @@ def test_parse_pmofn_rejects_invalid_index(m5stickv):
with pytest.raises(ValueError, match="Invalid pMofN part index"):
parse_pmofn_qr_part("p4of3 data")
+
+
+def test_detect_format_propagates_base_exceptions(mocker, m5stickv):
+ """detect_format catches genuine parsing errors (returns FORMAT_NONE) but
+ must NOT swallow BaseException-level signals like KeyboardInterrupt.
+
+ Regression for narrowing the bare except to ``except Exception``.
+ """
+ from krux.qr import detect_format
+
+ class Boom:
+ def startswith(self, _):
+ raise KeyboardInterrupt
+
+ with pytest.raises(KeyboardInterrupt):
+ detect_format(Boom())
+
+
+def test_max_qr_bytes_caps_at_last_version(mocker, m5stickv):
+ """A width beyond the supported version range falls back to the largest
+ capacity (exercises the narrowed `except IndexError`)."""
+ from krux.qr import max_qr_bytes, QR_CAPACITY_BYTE
+
+ assert max_qr_bytes(200) == QR_CAPACITY_BYTE[-1]
+
+
+def test_detect_format_returns_none_on_undecodable_data(mocker, m5stickv):
+ """Genuine parse errors (e.g. undecodable bytes) are caught and reported as
+ FORMAT_NONE (exercises the narrowed `except Exception`)."""
+ from krux.qr import detect_format, FORMAT_NONE
+
+ assert detect_format(b"\xff\xfe\xfd") == (FORMAT_NONE, None)
Why this scored 37/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.