Account for terminating '\0' in maximum length of ext_pubkey_str
What the developer wrote
Authored by Salvatore Ingala
Account for terminating '\0' in maximum length of ext_pubkey_str
What changed, and why it matters
This commit fixes a small but real off-by-one buffer sizing bug in the Ledger Bitcoin app's wallet code. A string buffer meant to hold a serialized extended public key was sized to the maximum character length, but C strings need an extra byte for the terminating null character ('\0'). Without that extra byte, code later reading or copying the string could read past the buffer or write the terminator into adjacent memory, which on a constrained hardware wallet could potentially leak secrets or cause a crash. The fix adds the missing +1 byte.
Review the rest of parse_policy_map_key_info() and any callers to confirm the string is now always null-terminated before use, and check whether similar MAX_SERIALIZED_PUBKEY_LENGTH-sized buffers elsewhere in the codebase have the same issue. Consider adding a static assertion or explicit terminator write to make the invariant obvious.
Security signals we found
Off-by-one buffer size missing null terminator
Potential stack buffer over-read or adjacent memory corruption
Extended public key parsing in hardware wallet code
No explicit bounds check on string termination visible in diff
Evidence from the diff
In src/common/wallet.c, parse_policy_map_key_info() declared ext_pubkey_str as char ext_pubkey_str[MAX_SERIALIZED_PUBKEY_LENGTH]. The function then reads up to MAX_SERIALIZED_PUBKEY_LENGTH alphanumeric characters into it and later likely treats it as a null-terminated C string (e.g., via base58 decoding or string helpers). Because no byte was reserved for the ‘\0’ terminator, a key of the maximum length would result in a non-null-terminated string or a terminator written one byte past the array. The patch changes the declaration to MAX_SERIALIZED_PUBKEY_LENGTH + 1, correcting the off-by-one. The loop guard still limits the read to MAX_SERIALIZED_PUBKEY_LENGTH characters, so the fix is safe and complete for this variable.
Changed components
src/common/wallet.cparse_policy_map_key_info()Extended public key string parsingInspect captured patch +1 / −1
diff --git a/src/common/wallet.c b/src/common/wallet.c
index 6fc2fef..828db09 100644
--- a/src/common/wallet.c
+++ b/src/common/wallet.c
@@ -360,7 +360,7 @@ int parse_policy_map_key_info(buffer_t *buffer, policy_map_key_info_t *out, int
// consume the rest of the buffer into the pubkey, except possibly the final "/**"
unsigned int ext_pubkey_len = 0;
- char ext_pubkey_str[MAX_SERIALIZED_PUBKEY_LENGTH];
+ char ext_pubkey_str[MAX_SERIALIZED_PUBKEY_LENGTH + 1];
uint8_t c;
while (ext_pubkey_len < MAX_SERIALIZED_PUBKEY_LENGTH && buffer_peek(buffer, &c) &&
is_alphanumeric(c)) {
Why this scored 58/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.