Make sure has_wallet_policy is not used uninitialized
What the developer wrote
Authored by Salvatore Ingala
Make sure has_wallet_policy is not used uninitialized
In the streaming case, a stale has_wallet_policy that is set to
true would lead to the previous wallet name being shown (if any).
What changed, and why it matters
This commit fixes a bug in the Ledger Bitcoin app where a flag indicating whether a wallet policy name should be shown could be left in an old/stale state during streaming transaction reviews. In practice, if a user had previously reviewed a transaction with a named wallet policy, a later transaction that should not show that name might incorrectly reuse the old name on screen. This is a user-interface consistency bug that could mislead the user about which wallet policy is authorizing a spend, but it does not directly expose private keys or allow unauthorized signing.
Review the broader transaction review state initialization to confirm all fields in g_ui_state are reset at the start of each streaming review, and verify that no other stale fields can influence user prompts or signing decisions. Consider adding regression tests for default-wallet-policy transactions following non-default ones.
Security signals we found
Uninitialized/stale state variable in security-critical UI flow
Wallet policy name displayed to user could be incorrect
Fix is defensive and partial (does not show full initialization context)
No direct cryptographic or signing bypass evident in diff
Evidence from the diff
The patch changes ui_prepare_authorize_wallet_spend() to accept NULL and explicitly set state->has_wallet_policy to false, and updates the caller in sign_psbt.c to pass NULL when st->is_wallet_default is true. Previously, the function was only called for non-default wallet policies, leaving has_wallet_policy potentially true from a prior streaming operation. The fix ensures the flag is initialized/reset for every transaction review, preventing stale wallet policy names from being displayed.
Changed components
src/handler/sign_psbt.csrc/ui/display.cui_validate_transaction_state_t state managementInspect captured patch +7 / −6
diff --git a/src/handler/sign_psbt.c b/src/handler/sign_psbt.c
index 211db5b..e192998 100644
--- a/src/handler/sign_psbt.c
+++ b/src/handler/sign_psbt.c
@@ -1455,9 +1455,7 @@ static bool __attribute__((noinline)) display_transaction(
// If it's not a default wallet policy, let's save this info to ask the user for
// confirmation
- if (!st->is_wallet_default) {
- ui_prepare_authorize_wallet_spend(st->wallet_header.name);
- }
+ ui_prepare_authorize_wallet_spend(!st->is_wallet_default ? st->wallet_header.name : NULL);
// "Review transaction to send Bitcoin"
if (!ui_transaction_streaming_prompt(dc)) {
diff --git a/src/ui/display.c b/src/ui/display.c
index 601ef67..407dbb8 100644
--- a/src/ui/display.c
+++ b/src/ui/display.c
@@ -200,9 +200,12 @@ bool ui_display_wallet_address(dispatcher_context_t *context,
void ui_prepare_authorize_wallet_spend(const char *wallet_name) {
ui_validate_transaction_state_t *state = (ui_validate_transaction_state_t *) &g_ui_state;
-
- strncpy(state->wallet_policy_name, wallet_name, sizeof(state->wallet_policy_name));
- state->has_wallet_policy = true;
+ if (wallet_name == NULL) {
+ state->has_wallet_policy = false;
+ } else {
+ strncpy(state->wallet_policy_name, wallet_name, sizeof(state->wallet_policy_name));
+ state->has_wallet_policy = true;
+ }
}
bool ui_warn_external_inputs(dispatcher_context_t *context) {
Why this scored 48/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.