Fixing call_get_merkleized_map_value() return value checking
What the developer wrote
Authored by Ilya Artemov
Fixing call_get_merkleized_map_value() return value checking
What changed, and why it matters
This commit fixes a bug in the Ledger Bitcoin app where several functions checked for only one specific error value (-1) instead of treating any negative number as a failure. If the underlying helper can return other negative error codes, the old code would have ignored them, potentially causing the app to keep processing with invalid or missing data. The patch makes the error handling more robust by rejecting any negative return value.
Review the implementation of call_get_merkleized_map_value() and related helpers to confirm all possible negative error codes, and verify that no other callers still use `== -1` checks. Consider adding unit tests that inject each documented error code. If any negative error code could previously reach signing logic, evaluate whether a security advisory is warranted.
Security signals we found
Incorrect error-code handling for negative return values
Potential bypass of failure path in PSBT parsing and signing
Defensive hardening of merkleized map value retrieval
Possible out-of-bounds or missing-data scenarios if non -1 negative codes are returned
Evidence from the diff
The patch changes six comparisons of the form result_len == -1 to result_len < 0 in callers of call_get_merkleized_map_value() (and related helpers). The function returns the length of a value on success and negative values on failure. The previous check only recognized -1 as failure, so any other negative error code would be misinterpreted as a valid length. Depending on the actual error codes the helper can produce, this could lead to out-of-bounds reads, incorrect PSBT parsing, or signing decisions based on incomplete data. The patch is defensive and correct, but the diff alone does not prove an exploitable path exists.
Changed components
src/handler/lib/policy.csrc/handler/sign_psbt.csrc/handler/sign_psbt/txhashes.cInspect captured patch +6 / −6
diff --git a/src/handler/lib/policy.c b/src/handler/lib/policy.c
index b8e9acc..6cd6613 100644
--- a/src/handler/lib/policy.c
+++ b/src/handler/lib/policy.c
@@ -439,7 +439,7 @@ __attribute__((noinline, warn_unused_result)) int get_extended_pubkey_from_clien
key_index,
(uint8_t *) key_info_str,
sizeof(key_info_str));
- if (key_info_len == -1) {
+ if (key_info_len < 0) {
return -1;
}
@@ -1799,7 +1799,7 @@ static int get_pubkey_from_merkle_tree(dispatcher_context_t *dispatcher_context,
index,
(uint8_t *) key_info_str,
sizeof(key_info_str));
- if (key_info_len == -1) {
+ if (key_info_len < 0) {
return WITH_ERROR(-1, "Failed to retrieve key info");
}
diff --git a/src/handler/sign_psbt.c b/src/handler/sign_psbt.c
index 9c8812d..2cd5157 100644
--- a/src/handler/sign_psbt.c
+++ b/src/handler/sign_psbt.c
@@ -278,7 +278,7 @@ init_global_state(dispatcher_context_t *dc, sign_psbt_state_t *st) {
1,
raw_result,
sizeof(raw_result));
- if (result_len == -1) {
+ if (result_len < 0) {
st->locktime = 0;
} else if (result_len != 4) {
SEND_SW(dc, SW_INCORRECT_DATA);
@@ -962,7 +962,7 @@ preprocess_outputs(dispatcher_context_t *dc,
output.in_out.scriptPubKey,
sizeof(output.in_out.scriptPubKey));
- if (result_len == -1 || result_len > (int) sizeof(output.in_out.scriptPubKey)) {
+ if (result_len < 0 || result_len > (int) sizeof(output.in_out.scriptPubKey)) {
SEND_SW(dc, SW_INCORRECT_DATA);
return false;
}
@@ -1272,7 +1272,7 @@ static bool get_output_script_and_amount(
out_scriptPubKey,
MAX_OUTPUT_SCRIPTPUBKEY_LEN);
- if (result_len == -1 || result_len > MAX_OUTPUT_SCRIPTPUBKEY_LEN) {
+ if (result_len < 0 || result_len > MAX_OUTPUT_SCRIPTPUBKEY_LEN) {
SEND_SW(dc, SW_INCORRECT_DATA);
return false;
}
diff --git a/src/handler/sign_psbt/txhashes.c b/src/handler/sign_psbt/txhashes.c
index 4849b00..400eb7c 100644
--- a/src/handler/sign_psbt/txhashes.c
+++ b/src/handler/sign_psbt/txhashes.c
@@ -98,7 +98,7 @@ static int hash_output_n(dispatcher_context_t *dc,
1,
out_script,
sizeof(out_script));
- if (out_script_len == -1) {
+ if (out_script_len < 0) {
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.