Merge pull request #249 from DaveGamble/fix-add-item

Release 1.7.4

CHANGELOG.md

@@ -1,3 +1,28 @@
+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:
+------
+* Fix potential double free, thanks @projectgus for reporting (see #241)
+
+1.7.2
+=====
+Fixes:
+------
+* Fix the use of GNUInstallDirs variables and the pkgconfig file. Thanks @zeerd for reporting (see #240)
+
+1.7.1
+=====
+Fixes:
+------
+* Fixed an Off-By-One error that could lead to an out of bounds write. Thanks @liuyunbin for reporting (see #230)
+* Fixed two errors with buffered printing. Thanks @liuyunbin for reporting (see #230)
+
 1.7.0
 =====
 Features:

CMakeLists.txt

@@ -7,7 +7,7 @@ include(GNUInstallDirs)
 
 set(PROJECT_VERSION_MAJOR 1)
 set(PROJECT_VERSION_MINOR 7)
-set(PROJECT_VERSION_PATCH 0)
+set(PROJECT_VERSION_PATCH 4)
 set(CJSON_VERSION_SO 1)
 set(CJSON_UTILS_VERSION_SO 1)
 set(PROJECT_VERSION "${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}")
@@ -107,12 +107,6 @@ endforeach()
 
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${supported_compiler_flags}")
 
-#variables for pkg-config
-set(prefix "${CMAKE_INSTALL_PREFIX}")
-set(libdir "${CMAKE_INSTALL_LIBDIR}")
-set(version "${PROJECT_VERSION}")
-set(includedir "${CMAKE_INSTALL_INCLUDEDIR}")
-
 option(BUILD_SHARED_LIBS "Build shared libraries" ON)
 option(ENABLE_TARGET_EXPORT "Enable exporting of CMake targets. Disable when it causes problems!" ON)
 
@@ -149,15 +143,15 @@ endif()
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/library_config/libcjson.pc.in"
     "${CMAKE_CURRENT_BINARY_DIR}/libcjson.pc" @ONLY)
 
-install(FILES cJSON.h DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/cjson")
-install (FILES "${CMAKE_CURRENT_BINARY_DIR}/libcjson.pc" DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig")
-install(TARGETS "${CJSON_LIB}" DESTINATION "${CMAKE_INSTALL_LIBDIR}" EXPORT "${CJSON_LIB}")
+install(FILES cJSON.h DESTINATION "${CMAKE_INSTALL_FULL_INCLUDEDIR}/cjson")
+install (FILES "${CMAKE_CURRENT_BINARY_DIR}/libcjson.pc" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}/pkgconfig")
+install(TARGETS "${CJSON_LIB}" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}" EXPORT "${CJSON_LIB}")
 if (BUILD_SHARED_AND_STATIC_LIBS)
-    install(TARGETS "${CJSON_LIB}-static" DESTINATION "${CMAKE_INSTALL_LIBDIR}")
+    install(TARGETS "${CJSON_LIB}-static" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}")
 endif()
 if(ENABLE_TARGET_EXPORT)
     # export library information for CMake projects
-    install(EXPORT "${CJSON_LIB}" DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/cJSON")
+    install(EXPORT "${CJSON_LIB}" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}/cmake/cJSON")
 endif()
 
 set_target_properties("${CJSON_LIB}"
@@ -188,15 +182,15 @@ if(ENABLE_CJSON_UTILS)
     configure_file("${CMAKE_CURRENT_SOURCE_DIR}/library_config/libcjson_utils.pc.in"
         "${CMAKE_CURRENT_BINARY_DIR}/libcjson_utils.pc" @ONLY)
 
-    install(TARGETS "${CJSON_UTILS_LIB}" DESTINATION "${CMAKE_INSTALL_LIBDIR}" EXPORT "${CJSON_UTILS_LIB}")
+    install(TARGETS "${CJSON_UTILS_LIB}" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}" EXPORT "${CJSON_UTILS_LIB}")
     if (BUILD_SHARED_AND_STATIC_LIBS)
-        install(TARGETS "${CJSON_UTILS_LIB}-static" DESTINATION "${CMAKE_INSTALL_LIBDIR}")
+        install(TARGETS "${CJSON_UTILS_LIB}-static" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}")
     endif()
-    install(FILES cJSON_Utils.h DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/cjson")
-    install (FILES "${CMAKE_CURRENT_BINARY_DIR}/libcjson_utils.pc" DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig")
+    install(FILES cJSON_Utils.h DESTINATION "${CMAKE_INSTALL_FULL_INCLUDEDIR}/cjson")
+    install (FILES "${CMAKE_CURRENT_BINARY_DIR}/libcjson_utils.pc" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}/pkgconfig")
     if(ENABLE_TARGET_EXPORT)
       # export library information for CMake projects
-      install(EXPORT "${CJSON_UTILS_LIB}" DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/cJSON")
+      install(EXPORT "${CJSON_UTILS_LIB}" DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}/cmake/cJSON")
     endif()
 
     set_target_properties("${CJSON_UTILS_LIB}"
@@ -216,7 +210,7 @@ configure_file(
 # Install package config files
 install(FILES ${PROJECT_BINARY_DIR}/cJSONConfig.cmake
     ${PROJECT_BINARY_DIR}/cJSONConfigVersion.cmake
-    DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/cJSON")
+    DESTINATION "${CMAKE_INSTALL_FULL_LIBDIR}/cmake/cJSON")
 
 option(ENABLE_CJSON_TEST "Enable building cJSON test" ON)
 if(ENABLE_CJSON_TEST)

Makefile

@@ -8,7 +8,7 @@ CJSON_TEST_SRC = cJSON.c test.c
 
 LDLIBS = -lm
 
-LIBVERSION = 1.7.0
+LIBVERSION = 1.7.4
 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 != 0)
+#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 4)
     #error cJSON.h and cJSON.c have different versions. Make sure that both have the same.
 #endif
 
@@ -512,7 +512,7 @@ static cJSON_bool print_number(const cJSON * const item, printbuffer * const out
     }
 
     /* reserve appropriate space in the output */
-    output_pointer = ensure(output_buffer, (size_t)length);
+    output_pointer = ensure(output_buffer, (size_t)length + sizeof(""));
     if (output_pointer == NULL)
     {
         return false;
@@ -1087,13 +1087,15 @@ CJSON_PUBLIC(cJSON *) cJSON_Parse(const char *value)
 
 static unsigned char *print(const cJSON * const item, cJSON_bool format, const internal_hooks * const hooks)
 {
+    static const size_t default_buffer_size = 256;
     printbuffer buffer[1];
     unsigned char *printed = NULL;
 
     memset(buffer, 0, sizeof(buffer));
 
     /* create buffer */
-    buffer->buffer = (unsigned char*) hooks->allocate(256);
+    buffer->buffer = (unsigned char*) hooks->allocate(default_buffer_size);
+    buffer->length = default_buffer_size;
     buffer->format = format;
     buffer->hooks = *hooks;
     if (buffer->buffer == NULL)
@@ -1111,7 +1113,7 @@ static unsigned char *print(const cJSON * const item, cJSON_bool format, const i
     /* check if reallocate is available */
     if (hooks->reallocate != NULL)
     {
-        printed = (unsigned char*) hooks->reallocate(buffer->buffer, buffer->length);
+        printed = (unsigned char*) hooks->reallocate(buffer->buffer, buffer->offset + 1);
         buffer->buffer = NULL;
         if (printed == NULL) {
             goto fail;
@@ -1309,10 +1311,6 @@ static cJSON_bool print_value(const cJSON * const item, printbuffer * const outp
             size_t raw_length = 0;
             if (item->valuestring == NULL)
             {
-                if (!output_buffer->noalloc)
-                {
-                    output_buffer->hooks.deallocate(output_buffer->buffer);
-                }
                 return false;
             }
 
@@ -1897,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 = (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;
-    }
+    item->string = new_key;
+    item->type = new_type;
 
     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 0
+#define CJSON_VERSION_PATCH 4
 
 #include <stddef.h>
 

library_config/cJSONConfig.cmake.in

@@ -2,8 +2,8 @@
 set(CJSON_UTILS_FOUND @ENABLE_CJSON_UTILS@)
 
 # The include directories used by cJSON
-set(CJSON_INCLUDE_DIRS "@prefix@/@includedir@")
-set(CJSON_INCLUDE_DIR "@prefix@/@includedir@")
+set(CJSON_INCLUDE_DIRS "@CMAKE_INSTALL_FULL_INCLUDEDIR@")
+set(CJSON_INCLUDE_DIR "@CMAKE_INSTALL_FULL_INCLUDEDIR@")
 
 get_filename_component(_dir "${CMAKE_CURRENT_LIST_FILE}" PATH)
 

library_config/libcjson.pc.in

@@ -1,9 +1,8 @@
-prefix=@prefix@
-libdir=${prefix}/@libdir@
-includedir=${prefix}/@includedir@
+libdir=@CMAKE_INSTALL_FULL_LIBDIR@
+includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@
 
 Name: libcjson
-Version: @version@
+Version: @PROJECT_VERSION@
 Description: Ultralightweight JSON parser in ANSI C
 URL: https://github.com/DaveGamble/cJSON
 Libs: -L${libdir} -lcjson

library_config/libcjson_utils.pc.in

@@ -1,9 +1,8 @@
-prefix=@prefix@
-libdir=${prefix}/@libdir@
-includedir=${prefix}/@includedir@
+libdir=@CMAKE_INSTALL_FULL_LIBDIR@
+includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@
 
 Name: libcjson_utils
-Version: @version@
+Version: @PROJECT_VERSION@
 Description: An implementation of JSON Pointer, Patch and Merge Patch based on cJSON.
 URL: https://github.com/DaveGamble/cJSON
 Libs: -L${libdir} -lcjson_utils

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();
 }