Move sign_psbt cache inside the handler
What the developer wrote
Authored by Salvatore Ingala
Move sign_psbt cache inside the handler
Using a global was a workaround around an old Nano X limitation on
the stack size, that has since been removed.
This reclaims about 2.5kb of memory from the .bss section, rather
allocating it on the stack during the sign_psbt flow.
What changed, and why it matters
This commit is a straightforward internal code cleanup: it moves a temporary data cache used during Bitcoin transaction signing from a permanent global variable into a local stack variable inside the signing function. The change frees about 2.5 KB of always-reserved RAM and is described by the developer as removing an old workaround for a device stack-size limit that no longer exists. There is no indication in the commit that this fixes a security bug or changes behavior visible to users or attackers.
No security action required. Treat as a normal maintainability/memory-optimization change. If reviewing for device-specific correctness, verify that the Nano X stack limit removal is accurate and that the local `cache` does not exceed available stack during the sign_psbt flow.
Security signals we found
No security-relevant keywords in commit title or message
No change to input parsing, cryptography, authorization, or protocol logic
Memory-layout refactor only: global .bss variable moved to function-local stack
No added bounds checks, input validation, or hardening
No vendor or researcher attribution to a vulnerability report
Evidence from the diff
The patch removes the global sign_psbt_cache_t G_sign_psbt_cache from the .bss section and instead declares sign_psbt_cache_t cache; as a local automatic variable inside handler_sign_psbt(). All call sites that previously took cache (a pointer to the global) now take &cache (address of the local). The fuzzing harness’s zero-symbols list is updated to remove G_sign_psbt_cache. The commit message explicitly frames this as a memory-layout refactor, not a security fix.
Changed components
src/handler/sign_psbt.cfuzzing/invariants/zero-symbols.txtInspect captured patch +6 / −14
### fuzzing/invariants/zero-symbols.txt
@@ -7,7 +7,6 @@ G_output_len
G_dispatcher_state
G_dispatcher_context
g_ui_state
-G_sign_psbt_cache
pairs
# Pointers overwritten by the harness every iteration (random bytes crash).
### src/handler/sign_psbt.c
@@ -41,13 +41,6 @@
#include "sw.h"
#include "txhashes.h"
-// We declare this in the global space in order to use less stack space, since BOLOS enforces on
-// some devices an 8kb stack limit.
-// Once this is resolved in BOLOS, we should move this to the function scope to avoid unnecessarily
-// reserving RAM that can only be used for the signing flow (which, at time of writing, is the most
-// RAM-intensive operation command of the app).
-sign_psbt_cache_t G_sign_psbt_cache;
-
void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) {
LOG_PROCESSOR(__FILE__, __LINE__, __func__);
@@ -62,8 +55,8 @@ void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) {
// read APDU inputs, initialize global state and read global PSBT map
if (!init_global_state(dc, &st)) return;
- sign_psbt_cache_t *cache = &G_sign_psbt_cache;
- init_sign_psbt_cache(cache);
+ sign_psbt_cache_t cache;
+ init_sign_psbt_cache(&cache);
// bitmap to keep track of which inputs are internal
uint8_t internal_inputs[BITVECTOR_REAL_SIZE(MAX_N_INPUTS_CAN_SIGN)];
@@ -81,14 +74,14 @@ void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) {
* - detect internal inputs that should be signed, and if there are external inputs or unusual
* sighashes
*/
- if (!preprocess_inputs(dc, &st, cache, internal_inputs)) return;
+ if (!preprocess_inputs(dc, &st, &cache, internal_inputs)) return;
/** OUTPUTS VERIFICATION FLOW
*
* For each output, check if it's a change address.
* Check if it's an acceptable output.
*/
- if (!preprocess_outputs(dc, &st, cache, internal_outputs)) return;
+ if (!preprocess_outputs(dc, &st, &cache, internal_outputs)) return;
// check if we're only executing the MuSig2 Round 1
bool only_signing_for_musig = true;
@@ -117,7 +110,7 @@ void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) {
// pubnonces; this does not involve the private keys, therefore we can do it without user
// confirmation
- if (!produce_musig2_pubnonces(dc, &st, &signing_state, cache, internal_inputs)) {
+ if (!produce_musig2_pubnonces(dc, &st, &signing_state, &cache, internal_inputs)) {
return;
}
}
@@ -152,7 +145,7 @@ void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) {
* For each internal key expression, and for each internal input, sign using the
* appropriate algorithm.
*/
- int sign_result = sign_transaction(dc, &st, cache, &signing_state, internal_inputs);
+ int sign_result = sign_transaction(dc, &st, &cache, &signing_state, internal_inputs);
#ifdef HAVE_SWAP
if (!G_called_from_swap)Why this scored 12/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.