Stricter bounds validation in int call_get_merkle_leaf_index
What the developer wrote
Authored by Salvatore Ingala
Stricter bounds validation in int call_get_merkle_leaf_index
What changed, and why it matters
This commit tightens validation in a Ledger Bitcoin app function that reads a Merkle tree leaf index from an external message. Previously, the function accepted any 64-bit unsigned index value. Now it rejects indexes larger than the maximum signed integer and indexes equal to or larger than the number of leaves. This prevents a malformed or hostile message from passing an out-of-range index into later code that expects a normal array position, which could otherwise cause incorrect behavior or memory corruption on a secure hardware wallet.
Treat this as a security hardening fix and include it in the next firmware release. Review all callers of `call_get_merkle_leaf_index()` to confirm they propagate the `-1` error and do not cast or truncate `index` elsewhere. Consider adding unit tests with boundary values (INT_MAX, INT_MAX+1, size-1, size, 0xFFFFFFFFFFFFFFFF) to prevent regression.
Security signals we found
Bounds check added for externally supplied varint index
Type-safety check against INT_MAX before likely signed-int use
Return -1 on validation failure consistent with existing error handling
Single-file, focused hardening change in security-critical hardware-wallet code
Evidence from the diff
In call_get_merkle_leaf_index(), the code reads a uint8_t found flag and a uint64_t index varint from the dispatcher read buffer. The patch adds two checks: index > INT_MAX and index >= (uint64_t) size. The first ensures the value fits in a signed integer, likely because downstream callers store or cast the index to int. The second ensures the index is within the actual Merkle tree leaf count. Without these checks, a malicious client could supply a very large varint that bypasses later bounds checks, potentially leading to out-of-bounds access or logic errors when the index is used to locate a leaf. The function returns -1 on failure, so callers must already handle error paths, but the missing validation was a clear defensive gap.
Changed components
src/handler/lib/get_merkle_leaf_index.ccall_get_merkle_leaf_index()Merkle leaf index parsing in client command handlerInspect captured patch +3 / −1
diff --git a/src/handler/lib/get_merkle_leaf_index.c b/src/handler/lib/get_merkle_leaf_index.c
index 8f220c6..e45168e 100644
--- a/src/handler/lib/get_merkle_leaf_index.c
+++ b/src/handler/lib/get_merkle_leaf_index.c
@@ -1,4 +1,5 @@
#include <string.h>
+#include <limits.h>
/* Local headers */
#include "client_commands.h"
@@ -27,7 +28,8 @@ int call_get_merkle_leaf_index(dispatcher_context_t *dispatcher_context,
uint64_t index;
if (!buffer_read_u8(&dispatcher_context->read_buffer, &found) ||
- !buffer_read_varint(&dispatcher_context->read_buffer, &index)) {
+ !buffer_read_varint(&dispatcher_context->read_buffer, &index) || index > INT_MAX ||
+ index >= (uint64_t) size) {
return -1;
}
Why this scored 61/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.