Fix heap-buffer-overflow in parse_string on truncated escapes

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)

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 <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#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;
+}