Improve error handling of copy_optional_string
What the developer wrote
Authored by Salvatore Ingala
Improve error handling of copy_optional_string
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
What changed, and why it matters
This commit tightens error handling in a unit-test helper that reads TOML configuration files. Previously, the helper treated any non-string value the same as a missing value, silently returning false. Now it explicitly distinguishes 'field is missing' (still returns false) from 'field exists but is the wrong type' (now logs an error and aborts). This is a hardening change in test infrastructure, not a fix for a demonstrated vulnerability in the Ledger app itself.
Treat as a minor hardening improvement in test tooling. Review whether any existing TOML test fixtures accidentally relied on the old silent-failure behavior, and confirm that abort() is acceptable for unit-test failures. No urgent security response is warranted based solely on this diff.
Security signals we found
Improper error handling / silent failure on unexpected input type
Test-only code path; no direct device-firmware or production code affected
Abort-on-error pattern introduced to fail fast on malformed test data
No evidence of memory corruption, buffer overflow, or cryptographic weakness in the diff
Evidence from the diff
The function copy_optional_string in unit-tests/libs/toml_helpers.h previously returned false whenever d.type != TOML_STRING, conflating absent/unknown fields with fields of an unexpected type. The patch splits the check: d.type == TOML_UNKNOWN returns false (missing/optional field), while any other non-string type triggers fprintf(stderr, …) and abort(). This prevents a malformed TOML test input from being silently interpreted as an absent optional string, which could mask test misconfigurations. The change is in unit-test support code, not device firmware or host production code.
Changed components
unit-tests/libs/toml_helpers.hcopy_optional_string helper functionInspect captured patch +5 / −1
diff --git a/unit-tests/libs/toml_helpers.h b/unit-tests/libs/toml_helpers.h
index 04910b3..17be303 100644
--- a/unit-tests/libs/toml_helpers.h
+++ b/unit-tests/libs/toml_helpers.h
@@ -49,9 +49,13 @@ static inline bool copy_optional_string(toml_datum_t table,
char *dst,
size_t dst_size) {
toml_datum_t d = toml_get(table, key);
- if (d.type != TOML_STRING) {
+ if (d.type == TOML_UNKNOWN) {
return false;
}
+ if (d.type != TOML_STRING) {
+ fprintf(stderr, "non-string field: %s\n", key);
+ abort();
+ }
if ((size_t) d.u.str.len >= dst_size) {
fprintf(stderr, "field %s too long (%d >= %zu)\n", key, d.u.str.len, dst_size);
abort();
Why this scored 24/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.