Add defensive check to silence clang static analyzer
What the developer wrote
Authored by Salvatore Ingala
Add defensive check to silence clang static analyzer
What changed, and why it matters
This commit adds two safety checks in the Bitcoin app's policy-handling code. The developer says these checks 'can never happen' in normal operation and were added mainly to satisfy a static-analysis tool (clang analyzer). In theory, if the impossible did happen, the code could have used an invalid array index; the patch now returns an error instead. There is no evidence this is exploitable in practice.
Treat as a minor hardening change. Review whether the invariant (that at least one unused key always exists when the loop completes) is truly guaranteed by all callers and prior validation, and consider adding an explicit assertion or test case. No urgent security response is warranted based solely on this diff.
Security signals we found
Defensive bounds/negative-index guard added
Comment claims guard is unreachable ('can never happen')
Motivated by static analyzer warning rather than reported bug
Potential out-of-bounds/undefined behavior if invariant were violated
Evidence from the diff
In src/handler/lib/policy.c, process_multi_sortedmulti_node() and process_multi_a_sortedmulti_a_node() each received a defensive if (smallest_pubkey_index < 0) guard before calling bitvector_set(…, smallest_pubkey_index, true). The variable is an int initialized to -1 and is expected to be set inside an inner loop over a bitvector of ‘used’ keys. The commit message frames the change as silencing the clang static analyzer. The guard prevents a negative index from being passed to bitvector_set, which would otherwise be undefined behavior / out-of-bounds access. The comment ‘// can never happen’ indicates the author believes the guard is unreachable under correct state.
Changed components
src/handler/lib/policy.cprocess_multi_sortedmulti_node()process_multi_a_sortedmulti_a_node()sortedmulti wallet policy handlingInspect captured patch +6 / −0
diff --git a/src/handler/lib/policy.c b/src/handler/lib/policy.c
index d12ec62..403b40b 100644
--- a/src/handler/lib/policy.c
+++ b/src/handler/lib/policy.c
@@ -888,6 +888,9 @@ __attribute__((warn_unused_result)) static int process_multi_sortedmulti_node(
}
}
}
+ if (smallest_pubkey_index < 0) {
+ return WITH_ERROR(-1, "No unused key found"); // can never happen
+ }
bitvector_set(used, smallest_pubkey_index, true); // mark the key as used
}
@@ -955,6 +958,9 @@ __attribute__((warn_unused_result)) static int process_multi_a_sortedmulti_a_nod
}
}
}
+ if (smallest_pubkey_index < 0) {
+ return WITH_ERROR(-1, "No unused key found"); // can never happen
+ }
bitvector_set(used, smallest_pubkey_index, true); // mark the key as used
}
Why this scored 21/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.