What changed, and why it matters
This commit fixes a bug in how SeedSigner reads setup information from a QR code. Previously, if a setting value was left blank (for example, 'sigs=' with nothing after the equals sign), the app would silently accept it and could misinterpret later settings. Now the app rejects blank values and reports an error. This is a defensive hardening change rather than a confirmed exploit, but it prevents potential misconfiguration or unexpected behavior.
Treat as a low-risk hardening fix. Review whether other malformed inputs (missing equals sign, unknown setting names, duplicate keys, or extra whitespace) are also handled safely, and consider adding tests for those cases.
Security signals we found
Input validation hardening for QR-parsed configuration
Explicit rejection of empty setting values that could lead to misconfiguration
Addition of regression test for malformed SettingsQR input
Evidence from the diff
The Settings.parse_settingsqr() method splits SettingsQR payload entries on whitespace using entry.split(‘=’). When a value was empty (e.g., ‘sigs=’), value became the empty string. The original code then treated ‘’ as a valid value, which could cause incorrect option validation or silently skip a setting. The patch adds an explicit guard: if value == ‘’, it raises InvalidSettingsQRData. A unit test verifies that ‘settings::v1 persistent=D sigs= camera=180’ raises an exception mentioning ‘sigs’.
Changed components
src/seedsigner/models/settings.pytests/test_settings.pyInspect captured patch +13 / −0
diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py
index 19eba89..d15b165 100644
--- a/src/seedsigner/models/settings.py
+++ b/src/seedsigner/models/settings.py
@@ -81,6 +81,10 @@ class Settings(Singleton):
for entry in data.split()[split_index:]:
abbreviated_name, value = entry.split("=")
+ # Empty values ("some_setting= other_setting=E") are invalid
+ if value == "":
+ raise InvalidSettingsQRData(f"{abbreviated_name} cannot be empty")
+
# Parse multi-value settings; integer-ize where needed
if "," in value:
values_updated = []
@@ -103,6 +107,7 @@ class Settings(Singleton):
values = [value]
else:
values = value
+
for v in values:
if v not in [opt[0] for opt in settings_entry.selection_options]:
if settings_entry.attr_name == SettingsConstants.SETTING__PERSISTENT_SETTINGS and v == SettingsConstants.OPTION__ENABLED:
diff --git a/tests/test_settings.py b/tests/test_settings.py
index 8cc391f..f896dca 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -108,6 +108,14 @@ class TestSettings(BaseTest):
assert "passphrase" in str(e.value)
+ def test_settingsqr_fails_empty_values(self):
+ """ SettingsQR parser should fail if a setting is empty """
+ settingsqr_data = "settings::v1 persistent=D sigs= camera=180"
+ with pytest.raises(InvalidSettingsQRData) as e:
+ Settings.parse_settingsqr(settingsqr_data)
+ assert "sigs" in str(e.value)
+
+
def test_settingsqr_parses_line_break_separators(self):
""" SettingsQR parser should read line breaks as acceptable separators """
settingsqr_data = "settings::v1\nname=Foo\nsigs=ss,ms\nscripts=nat,nes,tr\npassphrase=E\n"
Why this scored 36/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.