Refactor creation of static pairs struct
What the developer wrote
Authored by Salvatore Ingala
Refactor creation of static pairs struct
Writing the struct directly is fragile, and risks incomplete
initializations or non-zeroed fields 'leaking' into future calls.
We rather write a simple helper that creates the full struct with
a literal, guaranteeing that everything that is not explicitly set
is zeroed.
What changed, and why it matters
This commit is a defensive code cleanup in the user-interface layer of Ledger's Bitcoin app. It replaces repeated manual setup of a display data structure with a single helper function that always zeroes out any fields the caller does not explicitly set. The change reduces the risk that leftover data from a previous screen could accidentally leak into a later one, but it does not by itself fix a known, exploitable bug.
Treat as routine hardening. Review whether any other global UI structs in the codebase are partially initialized in the same way and consider applying the same pattern. No urgent security response is indicated by this commit alone.
Security signals we found
Defensive refactor to ensure full zero-initialization of a shared UI struct
Eliminates call sites that partially initialized `pairList`, which could leave stale fields
Only one field (`wrapping`) was previously set inconsistently across call sites; the helper now makes that explicit
No evidence in commit of an active exploit, CVE, or externally reported security issue
Evidence from the diff
The patch refactors initialization of the global pairList (nbgl_layoutTagValueList_t) in src/ui/display_nbgl.c. Previously, each call site set only the fields it cared about (pairs, nbPairs, nbMaxLinesForValue, wrapping), leaving any future-added fields or prior values unchanged. The new make_pair_list() helper creates the struct with a compound literal, so all unspecified members are zero-initialized. This is a hardening measure against information leakage or inconsistent UI state, not a patch for a reported vulnerability.
Changed components
src/ui/display_nbgl.cLedger Bitcoin app NBGL UI flows (transaction review, public key confirmation, address review, wallet policy registration, message signing)Inspect captured patch +19 / −49
diff --git a/src/ui/display_nbgl.c b/src/ui/display_nbgl.c
index b59ee5d..5e2352f 100644
--- a/src/ui/display_nbgl.c
+++ b/src/ui/display_nbgl.c
@@ -64,6 +64,17 @@ extern bool G_was_processing_screen_shown;
static void finish_transaction_flow(bool choice);
+static nbgl_layoutTagValueList_t *make_pair_list(unsigned int nbPairs, bool wrapping) {
+ pairList = (nbgl_layoutTagValueList_t) {
+ .pairs = pairs,
+ .nbPairs = nbPairs,
+ .nbMaxLinesForValue = 0,
+ .wrapping = wrapping,
+ };
+
+ return &pairList;
+}
+
// ux_flow_response
static void ux_flow_response_false(void) {
set_ux_flow_response(false);
@@ -149,9 +160,6 @@ void ui_display_transaction_simplified_flow_init(void) {
/* 1 From + MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER*3 + 1 Fees + 1 High fees */
_Static_assert(N_UX_PAIRS >= (1 + MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER * 3 + 1 + 1),
"Insufficient pairs for this flow");
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.pairs = pairs;
n_pairs = 0;
ui_validate_transaction_state_t *state = (ui_validate_transaction_state_t *) &g_ui_state;
@@ -203,10 +211,8 @@ void ui_display_transaction_simplified_flow_show(void) {
.value = state->fee,
.forcePageStart = state->n_outputs > 1 ? 1 : 0};
- pairList.nbPairs = n_pairs;
-
nbgl_useCaseReview(TYPE_TRANSACTION,
- &pairList,
+ make_pair_list(n_pairs, false),
&ICON_APP_ACTION,
GA_REVIEW_TRANSACTION,
NULL,
@@ -227,12 +233,8 @@ void ui_display_transaction_streaming_prompt(void) {
.item = "From",
.value = state->wallet_policy_name,
};
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.nbPairs = 1;
- pairList.pairs = pairs;
- nbgl_useCaseReviewStreamingContinue(&pairList, start_transaction_callback);
+ nbgl_useCaseReviewStreamingContinue(make_pair_list(1, false), start_transaction_callback);
}
}
@@ -248,19 +250,10 @@ void ui_display_transaction_streaming_output_address_amount(void) {
pairs[2].item = "Address";
pairs[2].value = state->address_or_description[0];
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.nbPairs = 3;
- pairList.pairs = pairs;
-
- nbgl_useCaseReviewStreamingContinue(&pairList, start_transaction_callback);
+ nbgl_useCaseReviewStreamingContinue(make_pair_list(3, false), start_transaction_callback);
}
void ui_display_transaction_streaming_flow(bool is_self_transfer) {
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.pairs = pairs;
-
unsigned int l_n_pairs = 0;
ui_validate_transaction_state_t *state = (ui_validate_transaction_state_t *) &g_ui_state;
@@ -274,19 +267,15 @@ void ui_display_transaction_streaming_flow(bool is_self_transfer) {
if (!is_self_transfer) {
pairs[l_n_pairs].item = "Fees";
pairs[l_n_pairs++].value = state->fee;
-
- pairList.nbPairs = l_n_pairs;
} else {
pairs[l_n_pairs].item = "Amount";
pairs[l_n_pairs++].value = "Self-transfer";
pairs[l_n_pairs].item = "Fees";
pairs[l_n_pairs++].value = state->fee;
-
- pairList.nbPairs = l_n_pairs;
}
- nbgl_useCaseReviewStreamingContinue(&pairList, finish_transaction_flow);
+ nbgl_useCaseReviewStreamingContinue(make_pair_list(l_n_pairs, false), finish_transaction_flow);
}
static void finish_transaction_flow(bool choice) {
@@ -309,13 +298,8 @@ void ui_display_pubkey_flow(void) {
pairs[1].item = "Public key";
pairs[1].value = g_ui_state.path_and_pubkey.pubkey;
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.nbPairs = 2;
- pairList.pairs = pairs;
-
nbgl_useCaseReviewLight(TYPE_OPERATION,
- &pairList,
+ make_pair_list(2, false),
&ICON_APP_ACTION,
"Confirm public key",
NULL,
@@ -328,13 +312,8 @@ void ui_display_receive_in_wallet_flow(void) {
pairs[0].item = "Account name";
pairs[0].value = g_ui_state.wallet.wallet_name;
- // Setup list
- pairList.nbMaxLinesForValue = 0;
- pairList.nbPairs = 1;
- pairList.pairs = pairs;
-
nbgl_useCaseAddressReview(g_ui_state.wallet.address,
- &pairList,
+ make_pair_list(1, false),
&ICON_APP_ACTION,
"Verify bitcoin\naddress",
NULL,
@@ -350,9 +329,6 @@ void ui_display_register_wallet_policy_flow(void) {
n_pairs = 0;
- pairList.nbMaxLinesForValue = 0;
- pairList.pairs = pairs;
-
pairs[n_pairs++] = (nbgl_layoutTagValue_t) {
.item = "Account name",
.value = g_ui_state.register_wallet_policy.wallet_name,
@@ -377,10 +353,8 @@ void ui_display_register_wallet_policy_flow(void) {
.value = g_ui_state.register_wallet_policy.keys_info[i]};
}
- pairList.nbPairs = n_pairs;
-
nbgl_useCaseReviewLight(TYPE_OPERATION,
- &pairList,
+ make_pair_list(n_pairs, false),
&ICON_APP_ACTION,
"Review account\nto register",
NULL,
@@ -404,12 +378,8 @@ void ui_sign_message_and_confirm_flow(bool is_hash) {
pairs[1].value = g_ui_state.path_and_message.message;
- pairList.wrapping = true;
- pairList.nbPairs = 2;
- pairList.pairs = pairs;
-
nbgl_useCaseReview(TYPE_MESSAGE,
- &pairList,
+ make_pair_list(2, true),
&ICON_APP_ACTION,
GA_REVIEW_MESSAGE,
NULL,
Why this scored 26/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.