Add missing bound check in call_get_preimage
What the developer wrote
Authored by Salvatore Ingala
Add missing bound check in call_get_preimage
Also add return value checks on all calls to buffer_write_bytes.
What changed, and why it matters
This commit fixes a missing safety check in the Ledger Bitcoin app's code that copies a 'preimage' (a piece of data used in Bitcoin transactions) into an output buffer. Before the fix, if the preimage was larger than the buffer, the app could write past the end of the buffer, corrupting memory. The patch adds a size check and also checks the result of every buffer write so the app stops safely instead of overflowing. This is a memory-safety bug that could potentially be exploited from a host computer talking to the device.
Treat this as a security fix and include it in the next firmware/app release. Review related handlers for similar unchecked buffer_write_bytes() calls. If a previous release shipped without this fix, consider whether an advisory is warranted for memory-corruption risk in a hardware wallet trusted code path.
Security signals we found
Missing bounds check on output buffer length
Unchecked return value of buffer_write_bytes leading to potential buffer overflow
Memory corruption risk in device-side APDU handler
Fix pattern consistent with CVE-worthy memory-safety defect
Evidence from the diff
In call_get_preimage(), the function now checks if (preimage_len > out_len) and returns -10 before any data is copied. Previously, buffer_write_bytes() calls were unchecked, so a large preimage_len could cause an out-of-bounds write on the output buffer. The patch also adds return-value checks to all buffer_write_bytes() calls in call_get_preimage() and call_get_merkle_preimage(), returning -11 on failure. The change is defensive and corrects a classic missing-bounds-check vulnerability pattern in a trusted execution environment (the Ledger Secure Element).
Changed components
src/handler/lib/get_preimage.csrc/handler/lib/get_merkle_preimage.cLedger Bitcoin app preimage/merkle preimage handlersInspect captured patch +20 / −4
diff --git a/src/handler/lib/get_merkle_preimage.c b/src/handler/lib/get_merkle_preimage.c
index fc1766a..6bb0de1 100644
--- a/src/handler/lib/get_merkle_preimage.c
+++ b/src/handler/lib/get_merkle_preimage.c
@@ -70,7 +70,11 @@ int call_get_merkle_preimage(dispatcher_context_t *dispatcher_context,
buffer_t out_buffer = buffer_create(out_ptr, out_ptr_len);
// write bytes to output
- buffer_write_bytes(&out_buffer, data_ptr + 1, partial_data_len - 1); // we skip the first byte
+ if (!buffer_write_bytes(&out_buffer,
+ data_ptr + 1, // we skip the first byte
+ partial_data_len - 1)) {
+ return -11;
+ }
size_t bytes_remaining = (size_t) preimage_len - partial_data_len;
@@ -105,7 +109,9 @@ int call_get_merkle_preimage(dispatcher_context_t *dispatcher_context,
crypto_hash_update(&hash_context.header, data_ptr, n_bytes);
// write bytes to output
- buffer_write_bytes(&out_buffer, data_ptr, n_bytes);
+ if (!buffer_write_bytes(&out_buffer, data_ptr, n_bytes)) {
+ return -11;
+ }
bytes_remaining -= n_bytes;
}
diff --git a/src/handler/lib/get_preimage.c b/src/handler/lib/get_preimage.c
index 357b932..24e72d9 100644
--- a/src/handler/lib/get_preimage.c
+++ b/src/handler/lib/get_preimage.c
@@ -43,6 +43,11 @@ int call_get_preimage(dispatcher_context_t *dispatcher_context,
return -4;
}
+ if (preimage_len > out_len) {
+ PRINTF("Output buffer too short\n");
+ return -10;
+ }
+
buffer_t buffer_out = buffer_create(out, out_len);
uint8_t *data_ptr =
@@ -54,7 +59,10 @@ int call_get_preimage(dispatcher_context_t *dispatcher_context,
crypto_hash_update(&hash_context.header, data_ptr, partial_data_len);
// write to output buffer
- buffer_write_bytes(&buffer_out, data_ptr, partial_data_len);
+
+ if (!buffer_write_bytes(&buffer_out, data_ptr, partial_data_len)) {
+ return -11;
+ }
size_t bytes_remaining = (size_t) preimage_len - partial_data_len;
@@ -91,7 +99,9 @@ int call_get_preimage(dispatcher_context_t *dispatcher_context,
// update hash
crypto_hash_update(&hash_context.header, data_ptr, n_bytes);
- buffer_write_bytes(&buffer_out, data_ptr, n_bytes);
+ if (!buffer_write_bytes(&buffer_out, data_ptr, n_bytes)) {
+ return -11;
+ }
bytes_remaining -= n_bytes;
}
Why this scored 72/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.