Fail immediately on overflow before converting from uint64_t to uint32_t
What the developer wrote
Authored by Salvatore Ingala
Fail immediately on overflow before converting from uint64_t to uint32_t
What changed, and why it matters
This commit fixes two spots in Ledger's Bitcoin app where a very large 64-bit number could be silently truncated to a smaller 32-bit number. After the truncation, the app would treat a huge requested data length as a small one, which could let an attacker read memory beyond what was intended or behave in unexpected ways. The fix now checks for overflow and fails immediately before the conversion happens.
Treat as a security hardening fix with possible memory-safety implications. Review whether the affected command handlers can be reached from untrusted host input, and assess whether a downstream advisory or CVE is warranted. Apply the patch and verify no other uint64_t-to-uint32_t length conversions lack bounds checks.
Security signals we found
Integer truncation / overflow in length conversion (uint64_t -> uint32_t)
Potential out-of-bounds read or logic bypass in preimage handling
Defensive length validation added before unsafe cast
No explicit CVE or security advisory referenced in commit
Evidence from the diff
In call_get_preimage() and call_stream_preimage(), a uint64_t preimage_len_u64 was cast to uint32_t without bounds checking. Values greater than UINT32_MAX would wrap modulo 2^32, producing a much smaller preimage_len. The patch adds an explicit overflow check (preimage_len_u64 > UINT32_MAX) and returns an error (-11 and -10 respectively) before the cast. This prevents integer truncation from being exploited to bypass subsequent length/validation logic.
Changed components
src/handler/lib/get_preimage.csrc/handler/lib/stream_preimage.cInspect captured patch +12 / −0
diff --git a/src/handler/lib/get_preimage.c b/src/handler/lib/get_preimage.c
index 24e72d9..2499b60 100644
--- a/src/handler/lib/get_preimage.c
+++ b/src/handler/lib/get_preimage.c
@@ -1,4 +1,5 @@
#include <string.h>
+#include <limits.h>
/* Local headers */
#include "client_commands.h"
@@ -32,6 +33,11 @@ int call_get_preimage(dispatcher_context_t *dispatcher_context,
!buffer_can_read(&dispatcher_context->read_buffer, partial_data_len)) {
return -2;
}
+
+ if (preimage_len_u64 > UINT32_MAX) {
+ return -11;
+ }
+
uint32_t preimage_len = (uint32_t) preimage_len_u64;
if (preimage_len < 1) {
diff --git a/src/handler/lib/stream_preimage.c b/src/handler/lib/stream_preimage.c
index bf25cc4..9f9fda0 100644
--- a/src/handler/lib/stream_preimage.c
+++ b/src/handler/lib/stream_preimage.c
@@ -1,4 +1,5 @@
#include <string.h>
+#include <limits.h>
#include "stream_preimage.h"
@@ -34,6 +35,11 @@ int call_stream_preimage(dispatcher_context_t *dispatcher_context,
!buffer_can_read(&dispatcher_context->read_buffer, partial_data_len)) {
return -2;
}
+
+ if (preimage_len_u64 > UINT32_MAX) {
+ return -10;
+ }
+
uint32_t preimage_len = (uint32_t) preimage_len_u64;
if (preimage_len < 1 || partial_data_len == 0) {
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.