common: implement str_to_u64, make json_to_u64 use it.
What the developer wrote
Authored by Rusty Russell
common: implement str_to_u64, make json_to_u64 use it.
The prior implementation could read past the end of the buffer (we actually
pad our JSON so this isn't harmful, but still). Fix up json_to_s64 and
json_to_double too, but since they're not used as often, just copy the
string there.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
What changed, and why it matters
This commit fixes a bug in how Core Lightning parsed unsigned 64-bit integers from JSON. The old code used a standard C library function (strtoull) that could read beyond the intended token boundary. The commit author notes this is not currently harmful because JSON buffers are padded, but it is still a defensive fix. The commit also applies similar safer handling to signed 64-bit and double parsing.
Treat as a low-severity hardening fix. Review whether any other JSON numeric parsers in the codebase rely on similar strto* pointer comparisons without copying or bounding input. No immediate emergency action is indicated because the author states JSON is padded, but the fix should be included in normal releases.
Security signals we found
Out-of-bounds read risk in JSON numeric parsing
Use of unbounded C library string-to-number conversion on non-null-terminated input
Defensive hardening of integer parsing helpers
Overflow checks added/reinforced for u64 parsing
Evidence from the diff
The patch introduces a new bounded parser str_to_u64() that only reads exactly buflen bytes and rejects non-digit characters and overflow. json_to_u64() is rewritten to use this helper. json_to_s64() and json_to_double() now copy the token to a null-terminated temporary buffer before calling strtoll()/strtod(), so the C library functions stop at the copied boundary rather than potentially scanning further in the original buffer. The old json_to_u64() relied on comparing the end pointer to buffer + tok->end, which assumes strtoull stops exactly at the token end; if the buffer were not padded or the token were malformed, this could read out of bounds or accept trailing data.
Changed components
common/json_parse_simple.ccommon/utils.ccommon/utils.hInspect captured patch +31 / −23
diff --git a/common/json_parse_simple.c b/common/json_parse_simple.c
index ce6ab606..7e0e299c 100644
--- a/common/json_parse_simple.c
+++ b/common/json_parse_simple.c
@@ -61,35 +61,19 @@ char *json_strdup(const tal_t *ctx, const char *buffer, const jsmntok_t *tok)
bool json_to_u64(const char *buffer, const jsmntok_t *tok, u64 *num)
{
- char *end;
- unsigned long long l;
-
- errno = 0;
- l = strtoull(buffer + tok->start, &end, 0);
- if (end != buffer + tok->end)
- return false;
-
- BUILD_ASSERT(sizeof(l) >= sizeof(*num));
- *num = l;
-
- /* Check for overflow */
- if (l == ULLONG_MAX && errno == ERANGE)
- return false;
-
- if (*num != l)
- return false;
-
- return true;
+ return str_to_u64(buffer + tok->start, tok->end - tok->start, num);
}
+/* Uncommon, we don't optimize these */
bool json_to_s64(const char *buffer, const jsmntok_t *tok, s64 *num)
{
+ const char *tmpbuf = json_strdup(tmpctx, buffer, tok);
char *end;
long long l;
errno = 0;
- l = strtoll(buffer + tok->start, &end, 0);
- if (end != buffer + tok->end)
+ l = strtoll(tmpbuf, &end, 0);
+ if (tmpbuf[0] == '\0' || *end != '\0')
return false;
BUILD_ASSERT(sizeof(l) >= sizeof(*num));
@@ -109,11 +93,12 @@ bool json_to_s64(const char *buffer, const jsmntok_t *tok, s64 *num)
bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num)
{
+ const char *tmpbuf = json_strdup(tmpctx, buffer, tok);
char *end;
errno = 0;
- *num = strtod(buffer + tok->start, &end);
- if (end != buffer + tok->end)
+ *num = strtod(tmpbuf, &end);
+ if (tmpbuf[0] == '\0' || *end != '\0')
return false;
/* Check for overflow/underflow */
diff --git a/common/utils.c b/common/utils.c
index 5dcbee5a..1a079900 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -253,3 +253,23 @@ void *membuf_tal_resize(struct membuf *mb, void *rawelems, size_t newsize)
tal_resize(&p, newsize);
return p;
}
+
+bool str_to_u64(const char *buf, size_t buflen, u64 *num)
+{
+ u64 val = 0;
+
+ if (buflen == 0)
+ return false;
+
+ for (size_t i = 0; i < buflen; i++) {
+ u64 digit;
+ if (buf[i] < '0' || buf[i] > '9')
+ return false;
+ digit = buf[i] - '0';
+ if (val > (UINT64_MAX - digit) / 10)
+ return false;
+ val = val * 10 + digit;
+ }
+ *num = val;
+ return true;
+}
diff --git a/common/utils.h b/common/utils.h
index 7a5c98aa..560ec9f5 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -119,6 +119,9 @@ static inline void tal_free_if_taken(const tal_t *p)
void tal_arr_append_(void *p, const void *append TAKES);
void tal_arr_appendn_(void *p, const void *append TAKES, size_t bytes);
+/* Parse a decimal u64 from exactly buflen bytes; false on bad chars or overflow */
+bool str_to_u64(const char *buf, size_t buflen, u64 *num);
+
/* Check for valid UTF-8 */
bool utf8_check(const void *buf, size_t buflen);
Why this scored 34/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.