Additional check to avoid vout_scriptpubkey overflow
What the developer wrote
Authored by Ilya Artemov
Additional check to avoid vout_scriptpubkey overflow
What changed, and why it matters
This commit adds a safety check in the Ledger Bitcoin app's code that parses previous transaction outputs. Before copying a scriptPubKey (the part of a Bitcoin output that locks funds) into a fixed-size buffer, the code now verifies the copy won't exceed the buffer's maximum length when combined with how much has already been written. Without this check, a malformed or oversized scriptPubKey could overflow the buffer, potentially corrupting memory and affecting the device's behavior. The commit also fixes two nearby variables from 'bool' to 'int' so parser error codes are preserved instead of being collapsed to true/false.
Treat this as a security-relevant hardening fix. Review whether the same offset-aware length check is needed elsewhere in the PSBT/raw transaction parser, confirm MAX_PREVOUT_SCRIPTPUBKEY_LEN is consistent with protocol limits, and consider fuzzing the parser with oversized and chunked scriptPubKeys. If a CVE is desired, request one through Ledger's disclosure process.
Security signals we found
Buffer overflow / out-of-bounds write prevention
Missing offset-aware bounds check before memcpy
Integer/boolean type correction that preserves error codes
PSBT / previous transaction parsing hardening
Evidence from the diff
In src/handler/lib/psbt_parse_rawtx.c, parse_rawtxoutput_scriptpubkey() now checks state->scriptpubkey_counter + data_len > MAX_PREVOUT_SCRIPTPUBKEY_LEN before calling memcpy() into vout_scriptpubkey. The prior length check only validated data_len against the maximum, ignoring the running offset, so a multi-chunk or resumed parse could write past the end of the fixed-size array. The commit also changes the return type of parser_run() results in parse_rawtx_inputs() and parse_rawtx_outputs() from bool to int, ensuring non-1 status codes (errors or stream exhaustion) propagate correctly instead of being truncated.
Changed components
src/handler/lib/psbt_parse_rawtx.cparse_rawtxoutput_scriptpubkey()parse_rawtx_inputs()parse_rawtx_outputs()vout_scriptpubkey bufferInspect captured patch +15 / −10
diff --git a/src/handler/lib/psbt_parse_rawtx.c b/src/handler/lib/psbt_parse_rawtx.c
index 1fd2fe4..d0c0ee4 100644
--- a/src/handler/lib/psbt_parse_rawtx.c
+++ b/src/handler/lib/psbt_parse_rawtx.c
@@ -244,6 +244,11 @@ static int parse_rawtxoutput_scriptpubkey(parse_rawtxoutput_state_t *state, buff
// MAX_PREVOUT_SCRIPTPUBKEY_LEN
}
+ if (state->scriptpubkey_counter + data_len > MAX_PREVOUT_SCRIPTPUBKEY_LEN) {
+ return -1; // exceeding MAX_PREVOUT_SCRIPTPUBKEY_LEN with scriptpubkey_counter
+ // offset
+ }
+
memcpy(state->parent_state->parser_outputs->vout_scriptpubkey +
state->scriptpubkey_counter,
data,
@@ -342,11 +347,11 @@ static int parse_rawtx_inputs_init(parse_rawtx_state_t *state, buffer_t *buffers
static int parse_rawtx_inputs(parse_rawtx_state_t *state, buffer_t *buffers[2]) {
while (state->in_counter < state->n_inputs) {
while (true) {
- bool result = parser_run(parse_rawtxinput_steps,
- n_parse_rawtxinput_steps,
- &state->input_parser_context,
- buffers,
- pic);
+ int result = parser_run(parse_rawtxinput_steps,
+ n_parse_rawtxinput_steps,
+ &state->input_parser_context,
+ buffers,
+ pic);
if (result != 1) {
return result; // stream exhausted, or error
} else {
@@ -384,11 +389,11 @@ static int parse_rawtx_outputs_init(parse_rawtx_state_t *state, buffer_t *buffer
static int parse_rawtx_outputs(parse_rawtx_state_t *state, buffer_t *buffers[2]) {
while (state->out_counter < state->n_outputs) {
while (true) {
- bool result = parser_run(parse_rawtxoutput_steps,
- n_parse_rawtxoutput_steps,
- &state->output_parser_context,
- buffers,
- pic);
+ int result = parser_run(parse_rawtxoutput_steps,
+ n_parse_rawtxoutput_steps,
+ &state->output_parser_context,
+ buffers,
+ pic);
if (result != 1) {
return result; // stream exhausted, or error
} else {
Why this scored 71/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.