High 71 Bitcoin Committed Feb 10, 2026 · 12:49 UTC

Fix issues with the communication protocol when data is chunked

0586ab2f View source ↗

Public commit record

What the developer wrote

Authored by Salvatore Ingala

73/100 · Adequate

Fix issues with the communication protocol when data is chunked

- get_merkle_preimage failed to update the data_ptr pointer
- fpt_der_data_callback would read corrupted data if a zero-length
chunk (or one starting with a very large varint that does not fit
in the first chunk) is received.

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context

The short version

What changed, and why it matters

This commit fixes two bugs in the Ledger Bitcoin app's data-receiving code when large messages are split into multiple chunks. One bug caused the app to hash and copy from the wrong memory location after the first chunk, potentially corrupting a cryptographic proof. The other bug could read corrupted data or behave incorrectly if an empty chunk arrived, or if a taproot PSBT field started with a very large variable-length integer. These are memory/data-integrity issues in security-critical signing code, but the commit does not state they are exploitable for theft.

Recommended action

Treat as a security-relevant bugfix and include in the next firmware/app release. Review other chunked-callback handlers for similar stale-pointer and empty-chunk issues. Add regression tests covering multi-chunk, empty-chunk, and boundary-condition inputs for both components. Consider whether the merkle_preimage corruption could affect transaction validation and assess for a coordinated disclosure if a practical exploit path is found.

Security signals we found

Memory/data pointer not updated across chunked reads

Potential hash of incorrect/corrupted preimage data

Zero-length chunk handling gap

Varint parsing across chunk boundaries replaced with fixed-size read

Off-by-one correction in memmove offset

Security-critical code path: PSBT BIP32 derivation extraction during signing

Technical analysis

Evidence from the diff

Two independent fixes in chunked communication handling. (1) In get_merkle_preimage.c, data_ptr was not updated after dispatcher_read_buffer refilled the read buffer, so subsequent crypto_hash_update and buffer_write_bytes calls reused the stale data_ptr from the first chunk, hashing/copying the wrong bytes. (2) In extract_bip32_derivation.c, fpt_der_data_callback now ignores zero-length chunks and replaces a varint read with a single-byte read for the taproot hash count, because a large varint that does not fit in the first chunk would otherwise be parsed incorrectly. A memmove offset is also corrected (out_data_length - d + 1 -> out_data_length - d).

Changed components

src/handler/lib/get_merkle_preimage.csrc/handler/sign_psbt/extract_bip32_derivation.c

Inspect captured patch +23 / −12

diff --git a/src/handler/lib/get_merkle_preimage.c b/src/handler/lib/get_merkle_preimage.c
index bee304f..6795208 100644
--- a/src/handler/lib/get_merkle_preimage.c
+++ b/src/handler/lib/get_merkle_preimage.c
@@ -97,11 +97,10 @@ int call_get_merkle_preimage(dispatcher_context_t *dispatcher_context,
             return -9;
         }
 
+        data_ptr = dispatcher_context->read_buffer.ptr + dispatcher_context->read_buffer.offset;
+
         // update hash
-        crypto_hash_update(
-            &hash_context.header,
-            dispatcher_context->read_buffer.ptr + dispatcher_context->read_buffer.offset,
-            n_bytes);
+        crypto_hash_update(&hash_context.header, data_ptr, n_bytes);
 
         // write bytes to output
         buffer_write_bytes(&out_buffer, data_ptr, n_bytes);
diff --git a/src/handler/sign_psbt/extract_bip32_derivation.c b/src/handler/sign_psbt/extract_bip32_derivation.c
index c034876..496a381 100644
--- a/src/handler/sign_psbt/extract_bip32_derivation.c
+++ b/src/handler/sign_psbt/extract_bip32_derivation.c
@@ -7,7 +7,6 @@
 
 #include "../../common/psbt.h"
 #include "../../common/read.h"
-#include "../../common/varint.h"
 
 typedef struct {
     int psbt_key_type;
@@ -27,6 +26,11 @@ static void fpt_der_data_callback(buffer_t *data, void *callback_state) {
 
     if (cs->result < 0) return;  // an error already happened, ignore the rest
 
+    if (data->size == 0) {
+        // ignore empty chunks
+        return;
+    }
+
     // on the first call, compute the length the fingerprint + derivation part of the message.
     // - if non-taproot, then it's the entire message;
     // - if taproot, it's the message after the hashes are removed.
@@ -37,16 +41,24 @@ static void fpt_der_data_callback(buffer_t *data, void *callback_state) {
         if (!is_tap) {
             cs->out_data_length = cs->total_data_length;
         } else {
-            uint64_t n_hashes;
-            if ((!buffer_read_varint(data, &n_hashes)) ||
-                (cs->total_data_length < varint_size(n_hashes) + 32 * (int) n_hashes)) {
+            // While BIP-0174 defines the number of hashes as a compact size integer, this
+            // can never be more than 128 in taproot. Since such numbers are always encoded
+            // as a single byte, we simplify by reading a single byte instead of a varint.
+            uint8_t n_hashes;
+            if ((!buffer_read_u8(data, &n_hashes)) ||
+                (cs->total_data_length < 1 + 32 * (int) n_hashes)) {
                 PRINTF("Unexpected: initial callback message too short\n");
                 cs->result = -1;
                 return;
             }
 
-            int out_data_length =
-                cs->total_data_length - varint_size(n_hashes) - 32 * (int) n_hashes;
+            if (n_hashes > 128) {
+                PRINTF("Unexpected: too many hashes in taproot BIP32 derivation\n");
+                cs->result = -1;
+                return;
+            }
+
+            int out_data_length = cs->total_data_length - 1 - 32 * (int) n_hashes;
 
             if (out_data_length > 4 * (1 + MAX_BIP32_PATH_STEPS)) {
                 PRINTF("BIP32 derivation longer than supported in psbt derivation\n");
@@ -69,9 +81,9 @@ static void fpt_der_data_callback(buffer_t *data, void *callback_state) {
         buffer_seek_set(data, 0);
         // We need to concatenate the new data we are reading with any previously read data.
         // Since we can only read data->size bytes, only the last d = out_data_length - data->size
-        // previous bytes are kept; they move from position out_data_length - d + 1 to position 0.
+        // previous bytes are kept; they move from position out_data_length - d to position 0.
         int d = cs->out_data_length - data->size;
-        memmove(cs->out, &cs->out[cs->out_data_length - d + 1], d);
+        memmove(cs->out, &cs->out[cs->out_data_length - d], d);
         // starting at position d, we read the entire data
         buffer_read_bytes(data, &cs->out[d], data->size);
     }

Risk score

Why this scored 71/100

Our methodology →

Potential impact 22/30

Exploitability 16/25

Stealth signal 10/15

Affected reach 12/15

Confidence 7/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 71/100

Community notes

Related watches

Propagate crypto_tr_lift_x errors in cpoint

Type consistency in psbt_parse_rawtx, and some other hardenings

Display total amount of external inputs when reliable