From e4c4d89b454bd3f989e167bc857aa0fe8eda1c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Afonso=20Janu=C3=A1rio?= Date: Fri, 18 Sep 2026 23:20:25 +0100 Subject: [PATCH] Fix heap-buffer-overflow in parse_string on truncated escapes parse_string() estimates the output buffer size in a first pass over the input, then decodes into a buffer of that size in a second pass. The \u escape handling in the second pass advanced its cursor by a fixed 4 (or 6, for a surrogate pair) characters without checking that the string actually had that many characters left. When the input contained an embedded NUL byte or simply ran out mid-escape, the cursor could jump past where the first pass had stopped counting, so the second pass kept copying bytes the allocation was never sized for and wrote past the end of it. A bare trailing backslash right at the end of an unterminated string had the same class of problem in the length-counting pass itself, skipping one character past the terminator and reading out of bounds on the next iteration. Both cases are now treated as malformed input: parse_string bails out and frees the partial buffer instead of reading or writing past it. Valid \u escapes, including surrogate pairs, are unaffected. Reported in #73 with an ASan repro from libFuzzer; this adds a regression test that reproduces the exact crash bytes plus a few related truncation cases, and checks that normal unicode escapes still round-trip correctly. --- cJSON.c | 34 +++++++- test/parse_string_overflow_test.c | 131 ++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 test/parse_string_overflow_test.c diff --git a/cJSON.c b/cJSON.c index b619181..3426eae 100644 --- a/cJSON.c +++ b/cJSON.c @@ -246,8 +246,9 @@ static const char *parse_string(cJSON *item, const char *str, const char **ep) } /* not a string! */ while (*ptr != '\"' && *ptr && ++len) - if (*ptr++ == '\\') - ptr++; /* Skip escaped quotes. */ + if (*ptr++ == '\\' && *ptr) + ptr++; /* Skip escaped quotes, but not past a trailing backslash + at the very end of the (unterminated) string. */ out = (char*) cJSON_malloc(len + 1); /* This is how long we need for the string, roughly. */ if (!out) @@ -262,6 +263,15 @@ static const char *parse_string(cJSON *item, const char *str, const char **ep) else { ptr++; + if (!*ptr) + { + /* Backslash was the last character before the string ran + out: there is nothing to escape. Bail out rather than + switching on (and then stepping past) the terminator. */ + cJSON_free(out); + *ep = ptr; + return 0; + } switch (*ptr) { case 'b': @@ -280,6 +290,18 @@ static const char *parse_string(cJSON *item, const char *str, const char **ep) *ptr2++ = '\t'; break; case 'u': /* transcode utf16 to utf8. */ + if (!isxdigit((unsigned char) ptr[1]) || !isxdigit((unsigned char) ptr[2]) + || !isxdigit((unsigned char) ptr[3]) || !isxdigit((unsigned char) ptr[4])) + { + /* \u escape is truncated (the string ended, possibly on an + embedded NUL byte, before four hex digits were found). + Bail out here instead of blindly skipping ahead by 4 + characters, which can walk ptr past the point the first + pass used to size out[] and overflow it below. */ + cJSON_free(out); + *ep = ptr; + return 0; + } sscanf(ptr + 1, "%4x", &uc); ptr += 4; /* get the unicode char. */ @@ -290,6 +312,14 @@ static const char *parse_string(cJSON *item, const char *str, const char **ep) { if (ptr[1] != '\\' || ptr[2] != 'u') break; // missing second-half of surrogate. + if (!isxdigit((unsigned char) ptr[3]) || !isxdigit((unsigned char) ptr[4]) + || !isxdigit((unsigned char) ptr[5]) || !isxdigit((unsigned char) ptr[6])) + { + /* Same truncation guard for the low surrogate half. */ + cJSON_free(out); + *ep = ptr; + return 0; + } sscanf(ptr + 3, "%4x", &uc2); ptr += 6; if (uc2 < 0xDC00 || uc2 > 0xDFFF) diff --git a/test/parse_string_overflow_test.c b/test/parse_string_overflow_test.c new file mode 100644 index 0000000..1ed621f --- /dev/null +++ b/test/parse_string_overflow_test.c @@ -0,0 +1,131 @@ +/* + * Regression test for a heap-buffer-overflow in parse_string() (cJSON.c). + * + * parse_string() first walks the input to estimate how many bytes the + * unescaped string will need, then walks it again to actually copy/decode + * the characters into a freshly malloc'd buffer sized from that estimate. + * + * The \u escape handler in the second pass used to advance its cursor by a + * fixed 4 (or 6, for a surrogate pair) characters without first checking + * that the string actually had that many characters left. If the source + * data contained an embedded NUL byte (or ran out) inside what looked like + * the start of a \u escape, the cursor could jump past the point where the + * first pass had stopped counting, letting the second pass copy trailing + * bytes the allocation was never sized for and overflow it. A bare + * trailing backslash right at the end of an (unterminated) string had the + * same problem in the length-counting pass itself. + * + * Build and run under AddressSanitizer to see the overflow reproduce + * against the old code, e.g.: + * + * clang -g -O0 -fsanitize=address,undefined \ + * -o parse_string_overflow_test \ + * test/parse_string_overflow_test.c cJSON.c + * ./parse_string_overflow_test + */ + +#include +#include +#include +#include + +#include "../cJSON.h" + +/* Parses a possibly-embedded-NUL byte buffer the same way callers that + accept arbitrary/untrusted input would: copy it into a heap buffer with + an explicit terminating NUL appended after the given length. */ +static cJSON *parse_bytes(const unsigned char *data, size_t len) { + char *buf = (char *) malloc(len + 1); + assert(buf != NULL); + memcpy(buf, data, len); + buf[len] = '\0'; + + const char *ep = NULL; + cJSON *item = cJSON_Parse(buf, &ep); + free(buf); + return item; +} + +static void test_reported_crash_input(void) { + /* Exact bytes from the originally reported fuzz crash: a JSON string + containing escaped quotes, a "\u" sequence, and an embedded NUL + byte before the (missing) closing quote. */ + static const unsigned char data[] = { + 0x22, 0x5c, 0x22, 0x5c, 0x22, 0x5c, 0x22, 0x5c, + 0x75, 0x65, 0x00, 0x0c, 0x5b, 0x75, 0x65, 0x5b + }; + + cJSON *item = parse_bytes(data, sizeof(data)); + /* The input is malformed (no closing quote, truncated \u escape), so + parsing must fail cleanly instead of overflowing the output buffer. */ + assert(item == NULL); +} + +static void test_truncated_unicode_escape(void) { + const char *inputs[] = { + "\"\\u12", /* only two hex digits before the string ends */ + "\"\\u", /* no hex digits at all */ + "\"\\uZZZZ\"", /* not hex digits */ + }; + for (size_t i = 0; i < sizeof(inputs) / sizeof(inputs[0]); i++) { + const char *ep = NULL; + cJSON *item = cJSON_Parse(inputs[i], &ep); + assert(item == NULL); + } +} + +static void test_truncated_surrogate_pair(void) { + /* Valid high surrogate followed by a low surrogate whose hex digits + are cut short. */ + const char *ep = NULL; + cJSON *item = cJSON_Parse("\"\\ud83d\\ude", &ep); + assert(item == NULL); +} + +static void test_trailing_backslash(void) { + const char *ep = NULL; + cJSON *item = cJSON_Parse("\"abc\\", &ep); + assert(item == NULL); +} + +/* Make sure the fix doesn't break correctly formed unicode escapes. */ +static void test_valid_unicode_escapes_still_work(void) { + struct { + const char *json; + const char *expected; + } cases[] = { + { "\"\\u00e9\"", "\xc3\xa9" }, /* e-acute, 2-byte utf8 */ + { "\"\\u20ac\"", "\xe2\x82\xac" }, /* euro sign, 3-byte utf8 */ + { "\"\\ud83d\\ude00\"", "\xf0\x9f\x98\x80" }, /* grinning face emoji */ + }; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const char *ep = NULL; + cJSON *item = cJSON_Parse(cases[i].json, &ep); + assert(item != NULL); + assert(item->valuestring != NULL); + assert(strcmp(item->valuestring, cases[i].expected) == 0); + cJSON_Delete(item); + } +} + +static void test_plain_and_object_still_work(void) { + const char *ep = NULL; + cJSON *item = cJSON_Parse("{\"a\":1,\"b\":\"x\\ny\"}", &ep); + assert(item != NULL); + cJSON *b = cJSON_GetObjectItem(item, "b"); + assert(b != NULL); + assert(strcmp(b->valuestring, "x\ny") == 0); + cJSON_Delete(item); +} + +int main(void) { + test_reported_crash_input(); + test_truncated_unicode_escape(); + test_truncated_surrogate_pair(); + test_trailing_backslash(); + test_valid_unicode_escapes_still_work(); + test_plain_and_object_still_work(); + printf("all parse_string overflow regression checks passed\n"); + return 0; +}