Correct allocation size for musig_info
What the developer wrote
Authored by Salvatore Ingala
Correct allocation size for musig_info
They both happen to be 4, so fortuitously not a bug on 32-bit ARM
What changed, and why it matters
A single-line fix in Ledger's Bitcoin app changes a memory allocation from using the size of a pointer to using the size of the actual data structure. On the 32-bit ARM hardware used by Ledger devices, both sizes happen to be 4 bytes, so the bug had no practical effect there. On a different architecture with larger pointers, the allocation would have been too small, potentially causing memory corruption when the structure is later written to. The commit message explicitly says this was not a bug on current 32-bit ARM devices.
Merge the fix; add a static-analysis or compiler warning check to catch sizeof(pointer) allocations; consider adding a regression test or build for 64-bit host simulator to detect similar issues. No urgent firmware release is required for 32-bit ARM devices, but the fix should be included in the next release cycle.
Security signals we found
Incorrect sizeof operand (pointer vs type) in memory allocation
Potential heap buffer overflow on non-32-bit architectures
MuSig key expression parsing code path affected
No active vulnerability on currently supported 32-bit ARM Ledger devices per commit message
Evidence from the diff
In src/common/wallet.c, parse_keyexpr allocated musig_aggr_key_info_t via buffer_alloc using sizeof(musig_info), where musig_info is a pointer. The correct expression is sizeof(musig_aggr_key_info_t). On 32-bit ARM, sizeof(void*) == sizeof(musig_aggr_key_info_t) == 4, so the allocation is coincidentally correct. On 64-bit or other platforms where pointers are larger than the struct, the allocation would be insufficient, leading to a heap overflow when the struct fields are populated. The patch is a defensive correctness fix with no demonstrated exploit path on shipped hardware.
Changed components
src/common/wallet.cparse_keyexpr functionmusig_aggr_key_info_t allocationLedger Bitcoin app MuSig key expression handlingInspect captured patch +1 / −1
diff --git a/src/common/wallet.c b/src/common/wallet.c
index 828db09..0f75579 100644
--- a/src/common/wallet.c
+++ b/src/common/wallet.c
@@ -521,7 +521,7 @@ static int parse_keyexpr(buffer_t *in_buf,
// allocate musig structures
musig_aggr_key_info_t *musig_info =
- (musig_aggr_key_info_t *) buffer_alloc(out_buf, sizeof(musig_info), true);
+ (musig_aggr_key_info_t *) buffer_alloc(out_buf, sizeof(musig_aggr_key_info_t), true);
if (musig_info == NULL) {
return WITH_ERROR(-1, "Out of memory");
Why this scored 49/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.