Release version 1.7.5

json patch: adding to a subfield of a non-object now fails as expected

CHANGELOG.md

@@ -1,4 +1,16 @@
-1.7.2
+1.7.5
+=====
+Fixes:
+------
+* Fix a bug in the JSON Patch implementation of `cJSON Utils` (see #251), thanks @bobkocisko.
+
+1.7.4
+=====
+Fixes:
+------
+* Fix potential use after free if the `string` parameter to `cJSON_AddItemToObject` is an alias of the `string` property of the object that is added (#248). Thanks @hhallen for reporting.
+
+1.7.3
 =====
 Fixes:
 ------

CMakeLists.txt

@@ -7,7 +7,7 @@ include(GNUInstallDirs)
 
 set(PROJECT_VERSION_MAJOR 1)
 set(PROJECT_VERSION_MINOR 7)
-set(PROJECT_VERSION_PATCH 3)
+set(PROJECT_VERSION_PATCH 5)
 set(CJSON_VERSION_SO 1)
 set(CJSON_UTILS_VERSION_SO 1)
 set(PROJECT_VERSION "${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}")

CONTRIBUTORS.md

@@ -7,6 +7,7 @@ Current Maintainer: [Max Bruckner](https://github.com/FSMaxB)
 * [Ajay Bhargav](https://github.com/ajaybhargav)
 * [Alper Akcan](https://github.com/alperakcan)
 * [Anton Sergeev](https://github.com/anton-sergeev)
+* [Bob Kocisko](https://github.com/bobkocisko)
 * [Christian Schulze](https://github.com/ChristianSch)
 * [Casperinous](https://github.com/Casperinous)
 * [Debora Grosse](https://github.com/DeboraG)
@@ -42,3 +43,5 @@ Current Maintainer: [Max Bruckner](https://github.com/FSMaxB)
 * [yangfl](https://github.com/yangfl)
 
 And probably more people on [SourceForge](https://sourceforge.net/p/cjson/bugs/search/?q=status%3Aclosed-rejected+or+status%3Aclosed-out-of-date+or+status%3Awont-fix+or+status%3Aclosed-fixed+or+status%3Aclosed&page=0)
+
+Also thanks to all the people who reported bugs and suggested new features.

Makefile

@@ -8,7 +8,7 @@ CJSON_TEST_SRC = cJSON.c test.c
 
 LDLIBS = -lm
 
-LIBVERSION = 1.7.3
+LIBVERSION = 1.7.5
 CJSON_SOVERSION = 1
 UTILS_SOVERSION = 1
 

cJSON.c

@@ -82,7 +82,7 @@ CJSON_PUBLIC(char *) cJSON_GetStringValue(cJSON *item) {
 }
 
 /* This is a safeguard to prevent copy-pasters from using incompatible C and header files */
-#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 3)
+#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 5)
     #error cJSON.h and cJSON.c have different versions. Make sure that both have the same.
 #endif
 
@@ -1895,32 +1895,37 @@ static void* cast_away_const(const void* string)
 
 static cJSON_bool add_item_to_object(cJSON * const object, const char * const string, cJSON * const item, const internal_hooks * const hooks, const cJSON_bool constant_key)
 {
+    char *new_key = NULL;
+    int new_type = cJSON_Invalid;
+
     if ((object == NULL) || (string == NULL) || (item == NULL))
     {
         return false;
     }
 
+    if (constant_key)
+    {
+        new_key = (char*)cast_away_const(string);
+        new_type = item->type | cJSON_StringIsConst;
+    }
+    else
+    {
+        new_key = (char*)cJSON_strdup((const unsigned char*)string, hooks);
+        if (new_key == NULL)
+        {
+            return false;
+        }
+
+        new_type = item->type & ~cJSON_StringIsConst;
+    }
+
     if (!(item->type & cJSON_StringIsConst) && (item->string != NULL))
     {
         hooks->deallocate(item->string);
     }
 
-    if (constant_key)
+    item->string = new_key;
-    {
+    item->type = new_type;
-        item->string = (char*)cast_away_const(string);
-        item->type |= cJSON_StringIsConst;
-    }
-    else
-    {
-        char *key = (char*)cJSON_strdup((const unsigned char*)string, hooks);
-        if (key == NULL)
-        {
-            return false;
-        }
-
-        item->string = key;
-        item->type &= ~cJSON_StringIsConst;
-    }
 
     return add_item_to_array(object, item);
 }

cJSON.h

@@ -31,7 +31,7 @@ extern "C"
 /* project version */
 #define CJSON_VERSION_MAJOR 1
 #define CJSON_VERSION_MINOR 7
-#define CJSON_VERSION_PATCH 3
+#define CJSON_VERSION_PATCH 5
 
 #include <stddef.h>
 

cJSON_Utils.c

@@ -988,6 +988,12 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_
         cJSON_AddItemToObject(parent, (char*)child_pointer, value);
         value = NULL;
     }
+    else /* parent is not an object */
+    {
+        /* Couldn't find object to add to. */
+        status = 9;
+        goto cleanup;
+    }
 
 cleanup:
     if (value != NULL)

tests/json-patch-tests/cjson-utils-tests.json

@@ -80,5 +80,12 @@
 	"doc": { "foo": ["bar"] },
 	"patch": [ { "op": "add", "path": "/foo/-", "value": ["abc", "def"] }],
 	"expected": {"foo": ["bar", ["abc", "def"]] }
-    }
+    },
+
+    { 
+        "comment": "15",
+        "doc": {"foo": {"bar": 1}},
+        "patch": [{"op": "add", "path": "/foo/bar/baz", "value": "5"}],
+        "error": "attempting to add to subfield of non-object"
+    } 
 ]

tests/misc_tests.c

@@ -508,6 +508,25 @@ static void cjson_create_array_reference_should_create_an_array_reference(void)
     cJSON_Delete(number_reference);
 }
 
+static void cjson_add_item_to_object_should_not_use_after_free_when_string_is_aliased(void)
+{
+    cJSON *object = cJSON_CreateObject();
+    cJSON *number = cJSON_CreateNumber(42);
+    char *name = (char*)cJSON_strdup((const unsigned char*)"number", &global_hooks);
+
+    TEST_ASSERT_NOT_NULL(object);
+    TEST_ASSERT_NOT_NULL(number);
+    TEST_ASSERT_NOT_NULL(name);
+
+    number->string = name;
+
+    /* The following should not have a use after free
+     * that would show up in valgrind or with AddressSanitizer */
+    cJSON_AddItemToObject(object, number->string, number);
+
+    cJSON_Delete(object);
+}
+
 int main(void)
 {
     UNITY_BEGIN();
@@ -530,6 +549,7 @@ int main(void)
     RUN_TEST(cjson_create_string_reference_should_create_a_string_reference);
     RUN_TEST(cjson_create_object_reference_should_create_an_object_reference);
     RUN_TEST(cjson_create_array_reference_should_create_an_array_reference);
+    RUN_TEST(cjson_add_item_to_object_should_not_use_after_free_when_string_is_aliased);
 
     return UNITY_END();
 }