descriptor: disallow leading zeros in descriptor numbers
What changed, and why it matters
This commit tightens the rules for numbers used in Bitcoin-style output descriptors, specifically for policy key placeholders like @0, @1, etc. It now rejects numbers with leading zeros (for example @00 or @01) and negative numbers with leading zeros. Such lax parsing can, in some contexts, allow two different-looking strings to refer to the same numeric key or trick downstream code that compares keys as text. The change is defensive and reduces the chance of descriptor ambiguity or canonicalization bugs.
Treat as a low-to-moderate hardening patch. Review whether any other descriptor number fields (key origin indexes, multi-path indexes, etc.) share the same strtoll_n() path and confirm they are also covered. No immediate emergency response is indicated, but the fix should be included in the next release.
Security signals we found
Input validation hardening in descriptor parser
Rejection of non-canonical numeric encodings (leading zeros)
Policy map key canonicalization improvement
Test coverage added for malformed policy key placeholders
Evidence from the diff
The patch modifies strtoll_n() in src/descriptor.c to reject (a) positive numbers with leading zeros and (b) negative numbers whose first digit after ‘-’ is ‘0’. It also updates a comment in is_valid_policy_map() to clarify that policy keys must be positive integers. Tests are added for negative keys, keys with leading space, and keys with leading zeros. The change is a hardening fix against non-canonical numeric encodings in descriptor policy maps.
Changed components
src/descriptor.c: strtoll_n()src/descriptor.c: is_valid_policy_map()src/test/test_descriptor.pyInspect captured patch +17 / −4
diff --git a/src/descriptor.c b/src/descriptor.c
index 59f1a24..d569fb8 100644
--- a/src/descriptor.c
+++ b/src/descriptor.c
@@ -310,7 +310,12 @@ static bool strtoll_n(const char *str, size_t str_len, int64_t *v)
char *end = NULL;
if (!str_len || str_len > sizeof(buf) - 1u ||
- (str[0] != '-' && (str[0] < '0' || str[0] > '9')))
+ /* Must start with '-' or a number */
+ (str[0] != '-' && (str[0] < '0' || str[0] > '9')) ||
+ /* Must not contain leading zeros */
+ (str[0] == '0' && str_len > 1) ||
+ /* Must not be negative and contain leading zeros */
+ (str[0] == '-' && str_len > 1 && str[1] == '0'))
return false; /* Too short/long, or invalid format */
memcpy(buf, str, str_len);
@@ -2893,9 +2898,10 @@ static bool is_valid_policy_map(const struct wally_map *map_in)
for (i = 0; ret == WALLY_OK && i < map_in->num_items; ++i) {
const struct wally_map_item *item = &map_in->items[i];
if (!item->key || item->key_len < 2 || item->key[0] != '@' ||
- !strtoll_n((const char *)item->key + 1, item->key_len - 1, &v) || v < 0)
- ret = WALLY_EINVAL; /* Policy keys can only be @n */
- else if ((size_t)v != i)
+ !strtoll_n((const char *)item->key + 1, item->key_len - 1, &v) || v < 0) {
+ /* Policy keys can only be @n: positive integers */
+ ret = WALLY_EINVAL;
+ } else if ((size_t)v != i)
ret = WALLY_EINVAL; /* Must be sorted in order from 0-n */
else if (!item->value || !item->value_len)
ret = WALLY_EINVAL; /* No key value */
diff --git a/src/test/test_descriptor.py b/src/test/test_descriptor.py
index 9ae38a5..b143901 100644
--- a/src/test/test_descriptor.py
+++ b/src/test/test_descriptor.py
@@ -370,6 +370,13 @@ class DescriptorTests(unittest.TestCase):
# Key multi-paths must be disjoint sets
[P|K, 'sh(multi(1,@0/<0;1>/*,@0/<1;2>/*))', [xpub1]],
[P|K, 'sh(multi(1,@0/<1;0>/*,@0/<2;1>/*))', [xpub1]],
+ # Keys must not be negative
+ [P, 'pkh(@-1/*)', {'@-1': xpub1}],
+ # Keys must not have leading space
+ [P, 'pkh(@ 0/*)', {'@ 0': xpub1}],
+ # Keys must not have leading zeros
+ [P, 'pkh(@00/*)', {'@00': xpub1}],
+ [P, 'sh(multi(1,@0/*,@01/*))', {'@0': xpub1, '@01': xpub2}],
]
d = c_void_p()
for flags, policy, key_items in bad_args:
Why this scored 37/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.