Ensure the policy_map_descriptor buffer is 0-terminated
What the developer wrote
Authored by Salvatore Ingala
Ensure the policy_map_descriptor buffer is 0-terminated
Reported by Cerberus
What changed, and why it matters
This commit fixes a buffer handling bug in the Ledger Bitcoin app's wallet registration code. The app now reserves one extra byte for the wallet policy descriptor string and explicitly writes a null terminator (the 'end of string' marker). Without this fix, code that reads the descriptor as a text string could keep reading past the intended data, potentially leaking nearby memory or behaving unpredictably. The issue was reported by an external party called Cerberus.
Review whether read_and_parse_wallet_policy() or its callers already enforce descriptor_template_len <= MAX_DESCRIPTOR_TEMPLATE_LENGTH, and confirm the +1 byte does not break any stack-size assumptions. Consider adding an explicit static assertion or runtime check that descriptor_template_len < sizeof(policy_map_descriptor).
Security signals we found
Buffer not null-terminated before string use
Potential out-of-bounds read / information disclosure
External security report credited (Cerberus)
Single-file, surgical patch in wallet registration handler
Evidence from the diff
In handler_register_wallet(), the local buffer policy_map_descriptor was sized exactly to MAX_DESCRIPTOR_TEMPLATE_LENGTH. The function read_and_parse_wallet_policy() fills it with up to wallet_header.descriptor_template_len bytes, but the buffer was not guaranteed to be NUL-terminated. By sizing the buffer to MAX_DESCRIPTOR_TEMPLATE_LENGTH + 1 and writing ‘\0’ at the byte after the parsed length, the commit ensures any downstream C-string consumers stop at the correct boundary. This is a defensive fix against out-of-bounds read / information disclosure via unterminated string handling.
Changed components
src/handler/register_wallet.chandler_register_wallet()policy_map_descriptor bufferInspect captured patch +2 / −1
diff --git a/src/handler/register_wallet.c b/src/handler/register_wallet.c
index fe1d684..eb6c61a 100644
--- a/src/handler/register_wallet.c
+++ b/src/handler/register_wallet.c
@@ -75,7 +75,7 @@ void handler_register_wallet(dispatcher_context_t *dc, uint8_t protocol_version)
return;
}
- uint8_t policy_map_descriptor[MAX_DESCRIPTOR_TEMPLATE_LENGTH];
+ uint8_t policy_map_descriptor[MAX_DESCRIPTOR_TEMPLATE_LENGTH + 1];
if (0 > read_and_parse_wallet_policy(dc,
&dc->read_buffer,
&wallet_header,
@@ -85,6 +85,7 @@ void handler_register_wallet(dispatcher_context_t *dc, uint8_t protocol_version)
SEND_SW(dc, SW_INCORRECT_DATA);
return;
}
+ policy_map_descriptor[wallet_header.descriptor_template_len] = '\0';
if (wallet_header.n_keys > MAX_N_KEYS_IN_WALLET_POLICY) {
PRINTF("At most %d key expressions are supported in a wallet policy.\n",
Why this scored 63/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.