fix(core/prodtest): call `cli_trace(cli, ...)` only if `cli` is not `NULL`
What the developer wrote
Authored by Ondřej Vejpustek
fix(core/prodtest): call `cli_trace(cli, ...)` only if `cli` is not `NULL`
[no changelog]
What changed, and why it matters
This commit fixes a programming bug in the Trezor hardware wallet's production-test code. The bug caused the device to call a debug/trace helper function even when no command-line interface was available, which could lead to a crash (null-pointer dereference) during production testing. The fix simply checks that the helper object exists before using it. This appears limited to internal production-test tooling and is unlikely to affect end-user wallets or real funds.
Treat as a low-severity hardening fix. Merge the patch. No urgent end-user action is indicated, because the affected code is gated behind the production-test build flag and is not part of normal device firmware operation.
Security signals we found
Null-pointer dereference in debug/trace instrumentation
Guard added around cli_trace calls under TREZOR_PRODTEST
No change to cryptographic session handling or key derivation logic
No changelog entry provided by vendor
Evidence from the diff
In core/embed/sec/tropic/tropic.c, multiple cli_trace(cli, ...) calls were made inside #if TREZOR_PRODTEST / #ifdef TREZOR_PRODTEST blocks without first verifying that cli was non-NULL. The patch adds if (cli) guards around each call. This prevents a null-pointer dereference / crash when these functions are invoked without a CLI context. The change is defensive and localized to prodtest instrumentation; no cryptographic or session logic is altered.
Changed components
core/embed/sec/tropic/tropic.cTropic secure-element integration codeTREZOR_PRODTEST production-test build pathInspect captured patch +26 / −11
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index dbac87ef..1563f132 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -146,8 +146,10 @@ static bool cache_tropic_cert_chain(cli_t *cli) {
ret = lt_get_info_cert_store(&g_tropic_driver.handle, &cert_store);
if (ret != LT_OK) {
#if TREZOR_PRODTEST
- cli_trace(cli, "lt_get_info_cert_store() failed with error '%s'",
- lt_ret_verbose(ret));
+ if (cli) {
+ cli_trace(cli, "lt_get_info_cert_store() failed with error '%s'",
+ lt_ret_verbose(ret));
+ }
#endif
return false;
}
@@ -155,8 +157,10 @@ static bool cache_tropic_cert_chain(cli_t *cli) {
ret = lt_get_st_pub(&cert_store, tropic_public_cached);
if (ret != LT_OK) {
#if TREZOR_PRODTEST
- cli_trace(cli, "lt_get_st_pub() failed with error '%s'",
- lt_ret_verbose(ret));
+ if (cli) {
+ cli_trace(cli, "lt_get_st_pub() failed with error '%s'",
+ lt_ret_verbose(ret));
+ }
#endif
return false;
}
@@ -200,7 +204,9 @@ bool tropic_wait_for_ready(cli_t *cli) {
if (!drv->initialized) {
#if TREZOR_PRODTEST
- cli_trace(cli, "Tropic driver is not initialized");
+ if (cli) {
+ cli_trace(cli, "Tropic driver is not initialized");
+ }
#endif
return false;
}
@@ -220,7 +226,9 @@ bool tropic_wait_for_ready(cli_t *cli) {
}
#if TREZOR_PRODTEST
- cli_trace(cli, "Tropic is busy");
+ if (cli) {
+ cli_trace(cli, "Tropic is busy");
+ }
#endif
return false;
}
@@ -257,7 +265,9 @@ lt_ret_t tropic_custom_session_start(cli_t *cli,
case TROPIC_PRIVILEGED_PAIRING_KEY_SLOT:
if (secret_key_tropic_pairing_privileged(trezor_private) != sectrue) {
#ifdef TREZOR_PRODTEST
- cli_trace(cli, "`secret_key_tropic_pairing_privileged()` failed");
+ if (cli) {
+ cli_trace(cli, "`secret_key_tropic_pairing_privileged()` failed");
+ }
#endif
goto cleanup;
}
@@ -265,7 +275,9 @@ lt_ret_t tropic_custom_session_start(cli_t *cli,
case TROPIC_UNPRIVILEGED_PAIRING_KEY_SLOT:
if (secret_key_tropic_pairing_unprivileged(trezor_private) != sectrue) {
#ifdef TREZOR_PRODTEST
- cli_trace(cli, "`secret_key_tropic_pairing_unprivileged()` failed");
+ if (cli) {
+ cli_trace(cli, "`secret_key_tropic_pairing_unprivileged()` failed");
+ }
#endif
goto cleanup;
}
@@ -285,8 +297,11 @@ lt_ret_t tropic_custom_session_start(cli_t *cli,
#endif
{
#ifdef TREZOR_PRODTEST
- cli_trace(cli,
- "`secret_key_tropic_public()` or `tropic_get_pubkey()` failed");
+ if (cli) {
+ cli_trace(
+ cli,
+ "`secret_key_tropic_public()` or `tropic_get_pubkey()` failed");
+ }
#endif
goto cleanup;
}
@@ -298,7 +313,7 @@ lt_ret_t tropic_custom_session_start(cli_t *cli,
pairing_key_index, trezor_private,
trezor_public));
#if TREZOR_PRODTEST
- if (ret != LT_OK) {
+ if (ret != LT_OK && cli) {
cli_trace(cli,
"`lt_session_start()` failed for pairing key %d "
"with error '%s'",
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.