Wrong counter increment

This reverts commit 5b502cdbfb.

Related to #860

CHANGELOG.md

@@ -1,3 +1,20 @@
+1.7.18 (May 13, 2024)
+======
+Fixes:
+------
+* Add NULL check to cJSON_SetValuestring()(CVE-2024-31755), see #839 and #840
+* Remove non-functional list handling of compiler flags, see #851
+* Fix heap buffer overflow, see #852
+* remove misused optimization flag -01, see #854
+* Set free'd pointers to NULL whenever they are not reassigned immediately after, see #855 and #833
+
+1.7.17 (Dec 26, 2023)
+======
+Fixes:
+------
+* Fix null reference in cJSON_SetValuestring(CVE-2023-50472), see #809
+* Fix null reference in cJSON_InsertItemInArray(CVE-2023-50471), see #809 and #810
+
 1.7.16 (Jul 5, 2023)
 ======
 Features:

CMakeLists.txt

@@ -2,7 +2,7 @@ set(CMAKE_LEGACY_CYGWIN_WIN32 0)
 cmake_minimum_required(VERSION 3.0)
 
 project(cJSON
-    VERSION 1.7.16
+    VERSION 1.7.18
     LANGUAGES C)
 
 cmake_policy(SET CMP0054 NEW)  # set CMP0054 policy
@@ -70,7 +70,6 @@ if (ENABLE_SANITIZERS)
         -fsanitize=float-cast-overflow
         -fsanitize-address-use-after-scope
         -fsanitize=integer
-        -01
         -fno-sanitize-recover
         )
 endif()
@@ -102,13 +101,10 @@ foreach(compiler_flag ${custom_compiler_flags})
 
     CHECK_C_COMPILER_FLAG(${compiler_flag} "FLAG_SUPPORTED_${current_variable}")
     if (FLAG_SUPPORTED_${current_variable})
-        list(APPEND supported_compiler_flags)
         set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${compiler_flag}")
     endif()
 endforeach()
 
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${supported_compiler_flags}")
-
 option(BUILD_SHARED_LIBS "Build shared libraries" ON)
 option(ENABLE_TARGET_EXPORT "Enable exporting of CMake targets. Disable when it causes problems!" ON)
 

CONTRIBUTORS.md

@@ -45,6 +45,8 @@ Contributors:
 * [Kevin Sapper](https://github.com/sappo)
 * [Kyle Chisholm](https://github.com/ChisholmKyle)
 * [Linus Wallgren](https://github.com/ecksun)
+* [Luo Jin](https://github.com/Up-wind)
+* [Max](https://github.com/maebex)
 * [MaxBrandtner](https://github.com/MaxBrandtner)
 * [Mateusz Szafoni](https://github.com/raiden00pl)
 * Mike Pontillo
@@ -55,6 +57,7 @@ Contributors:
 * [Moorthy](https://github.com/moorthy-bs)
 * [myd7349](https://github.com/myd7349)
 * [NancyLi1013](https://github.com/NancyLi1013)
+* [Orri](https://github.com/sbvoxel)
 * Paulo Antonio Alvarez
 * [Paweł Malowany](https://github.com/PawelMalowany)
 * [Pawel Winogrodzki](https://github.com/PawelWMS)

Makefile

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

SECURITY.md

@@ -0,0 +1,11 @@
+# Security Policy
+
+## Supported Versions
+
+Security is of the highest importance and all security vulnerabilities or suspected security vulnerabilities should be reported to cjson team privately, to minimize attacks against current users of cjson before they are fixed. Vulnerabilities will be investigated and patched on the next patch (or minor) release as soon as possible. This information could be kept entirely internal to the project.
+
+## Reporting a Vulnerability
+
+If you know of a publicly disclosed security vulnerability for cjson, please IMMEDIATELY contact wp_scut@163.com and peterlee@apache.org to inform the cjson Team.
+
+IMPORTANT: Do not file public issues on GitHub for security vulnerabilities.

cJSON.c

@@ -117,7 +117,7 @@ CJSON_PUBLIC(double) cJSON_GetNumberValue(const cJSON * const 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 != 16)
+#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 18)
     #error cJSON.h and cJSON.c have different versions. Make sure that both have the same.
 #endif
 
@@ -263,10 +263,12 @@ CJSON_PUBLIC(void) cJSON_Delete(cJSON *item)
         if (!(item->type & cJSON_IsReference) && (item->valuestring != NULL))
         {
             global_hooks.deallocate(item->valuestring);
+            item->valuestring = NULL;
         }
         if (!(item->type & cJSON_StringIsConst) && (item->string != NULL))
         {
             global_hooks.deallocate(item->string);
+            item->string = NULL;
         }
         global_hooks.deallocate(item);
         item = next;
@@ -397,16 +399,33 @@ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number)
     return object->valuedouble = number;
 }
 
+/* Note: when passing a NULL valuestring, cJSON_SetValuestring treats this as an error and return NULL */
 CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring)
 {
     char *copy = NULL;
+    size_t v1_len;
+    size_t v2_len;
     /* if object's type is not cJSON_String or is cJSON_IsReference, it should not set valuestring */
-    if (!(object->type & cJSON_String) || (object->type & cJSON_IsReference))
+    if ((object == NULL) || !(object->type & cJSON_String) || (object->type & cJSON_IsReference))
     {
         return NULL;
     }
-    if (strlen(valuestring) <= strlen(object->valuestring))
+    /* return NULL if the object is corrupted or valuestring is NULL */
+    if (object->valuestring == NULL || valuestring == NULL)
     {
+        return NULL;
+    }
+
+    v1_len = strlen(valuestring);
+    v2_len = strlen(object->valuestring);
+
+    if (v1_len <= v2_len)
+    {
+        /* strcpy does not handle overlapping string: [X1, X2] [Y1, Y2] => X2 < Y1 or Y2 < X1 */
+        if (!( valuestring + v1_len < object->valuestring || object->valuestring + v2_len < valuestring ))
+        {
+            return NULL;
+        }
         strcpy(object->valuestring, valuestring);
         return object->valuestring;
     }
@@ -888,6 +907,7 @@ fail:
     if (output != NULL)
     {
         input_buffer->hooks.deallocate(output);
+        output = NULL;
     }
 
     if (input_pointer != NULL)
@@ -1230,6 +1250,7 @@ static unsigned char *print(const cJSON * const item, cJSON_bool format, const i
 
         /* free the buffer */
         hooks->deallocate(buffer->buffer);
+        buffer->buffer = NULL;
     }
 
     return printed;
@@ -1238,11 +1259,13 @@ fail:
     if (buffer->buffer != NULL)
     {
         hooks->deallocate(buffer->buffer);
+        buffer->buffer = NULL;
     }
 
     if (printed != NULL)
     {
         hooks->deallocate(printed);
+        printed = NULL;
     }
 
     return NULL;
@@ -1283,6 +1306,7 @@ CJSON_PUBLIC(char *) cJSON_PrintBuffered(const cJSON *item, int prebuffer, cJSON
     if (!print_value(item, &p))
     {
         global_hooks.deallocate(p.buffer);
+        p.buffer = NULL;
         return NULL;
     }
 
@@ -1654,6 +1678,11 @@ static cJSON_bool parse_object(cJSON * const item, parse_buffer * const input_bu
             current_item = new_item;
         }
 
+        if (cannot_access_at_index(input_buffer, 1))
+        {
+            goto fail; /* nothing comes after the comma */
+        }
+
         /* parse the name of the child */
         input_buffer->offset++;
         buffer_skip_whitespace(input_buffer);
@@ -2186,7 +2215,7 @@ CJSON_PUBLIC(cJSON*) cJSON_AddArrayToObject(cJSON * const object, const char * c
 
 CJSON_PUBLIC(cJSON *) cJSON_DetachItemViaPointer(cJSON *parent, cJSON * const item)
 {
-    if ((parent == NULL) || (item == NULL))
+    if ((parent == NULL) || (item == NULL) || (item != parent->child && item->prev == NULL))
     {
         return NULL;
     }
@@ -2264,7 +2293,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_InsertItemInArray(cJSON *array, int which, cJSON
 {
     cJSON *after_inserted = NULL;
 
-    if (which < 0)
+    if (which < 0 || newitem == NULL)
     {
         return false;
     }
@@ -2275,6 +2304,11 @@ CJSON_PUBLIC(cJSON_bool) cJSON_InsertItemInArray(cJSON *array, int which, cJSON
         return add_item_to_array(array, newitem);
     }
 
+    if (after_inserted != array->child && after_inserted->prev == NULL) {
+        /* return false if after_inserted is a corrupted array item */
+        return false;
+    }
+
     newitem->next = after_inserted;
     newitem->prev = after_inserted->prev;
     after_inserted->prev = newitem;
@@ -2703,7 +2737,14 @@ CJSON_PUBLIC(cJSON *) cJSON_CreateStringArray(const char *const *strings, int co
 }
 
 /* Duplication */
+cJSON * cJSON_Duplicate_rec(const cJSON *item, size_t depth, cJSON_bool recurse);
+
 CJSON_PUBLIC(cJSON *) cJSON_Duplicate(const cJSON *item, cJSON_bool recurse)
+{
+    return cJSON_Duplicate_rec(item, 0, recurse );
+}
+
+cJSON * cJSON_Duplicate_rec(const cJSON *item, size_t depth, cJSON_bool recurse)
 {
     cJSON *newitem = NULL;
     cJSON *child = NULL;
@@ -2750,7 +2791,10 @@ CJSON_PUBLIC(cJSON *) cJSON_Duplicate(const cJSON *item, cJSON_bool recurse)
     child = item->child;
     while (child != NULL)
     {
-        newchild = cJSON_Duplicate(child, true); /* Duplicate (with recurse) each item in the ->next chain */
+        if(depth >= CJSON_CIRCULAR_LIMIT) {
+            goto fail;
+        }
+        newchild = cJSON_Duplicate_rec(child, depth + 1, true); /* Duplicate (with recurse) each item in the ->next chain */
         if (!newchild)
         {
             goto fail;
@@ -3116,4 +3160,5 @@ CJSON_PUBLIC(void *) cJSON_malloc(size_t size)
 CJSON_PUBLIC(void) cJSON_free(void *object)
 {
     global_hooks.deallocate(object);
+    object = NULL;
 }

cJSON.h

@@ -81,7 +81,7 @@ then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJ
 /* project version */
 #define CJSON_VERSION_MAJOR 1
 #define CJSON_VERSION_MINOR 7
-#define CJSON_VERSION_PATCH 16
+#define CJSON_VERSION_PATCH 18
 
 #include <stddef.h>
 
@@ -137,6 +137,12 @@ typedef int cJSON_bool;
 #define CJSON_NESTING_LIMIT 1000
 #endif
 
+/* Limits the length of circular references can be before cJSON rejects to parse them.
+ * This is to prevent stack overflows. */
+#ifndef CJSON_CIRCULAR_LIMIT
+#define CJSON_CIRCULAR_LIMIT 10000
+#endif
+
 /* returns the version of cJSON as a string */
 CJSON_PUBLIC(const char*) cJSON_Version(void);
 

tests/cjson_add.c

@@ -34,7 +34,7 @@ static void * CJSON_CDECL failing_malloc(size_t size)
     return NULL;
 }
 
-/* work around MSVC error C2322: '...' address of dillimport '...' is not static */
+/* work around MSVC error C2322: '...' address of dllimport '...' is not static */
 static void CJSON_CDECL normal_free(void *pointer)
 {
     free(pointer);

tests/misc_tests.c

@@ -219,6 +219,23 @@ static void cjson_should_not_parse_to_deeply_nested_jsons(void)
     TEST_ASSERT_NULL_MESSAGE(cJSON_Parse(deep_json), "To deep JSONs should not be parsed.");
 }
 
+static void cjson_should_not_follow_too_deep_circular_references(void)
+{
+    cJSON *o = cJSON_CreateArray();
+    cJSON *a = cJSON_CreateArray();
+    cJSON *b = cJSON_CreateArray();
+    cJSON *x;
+
+    cJSON_AddItemToArray(o, a);
+    cJSON_AddItemToArray(a, b);
+    cJSON_AddItemToArray(b, o);
+
+    x = cJSON_Duplicate(o, 1);
+    TEST_ASSERT_NULL(x);
+    cJSON_DetachItemFromArray(b, 0);
+    cJSON_Delete(o);
+}
+
 static void cjson_set_number_value_should_set_numbers(void)
 {
     cJSON number[1] = {{NULL, NULL, NULL, cJSON_Number, NULL, 0, 0, NULL}};
@@ -280,6 +297,21 @@ static void cjson_detach_item_via_pointer_should_detach_items(void)
     TEST_ASSERT_NULL_MESSAGE(parent->child, "Child of the parent wasn't set to NULL.");
 }
 
+static void cjson_detach_item_via_pointer_should_return_null_if_item_prev_is_null(void)
+{
+    cJSON list[2];
+    cJSON parent[1];
+
+    memset(list, '\0', sizeof(list));
+
+    /* link the list */
+    list[0].next = &(list[1]);
+
+    parent->child = &list[0];
+    TEST_ASSERT_NULL_MESSAGE(cJSON_DetachItemViaPointer(parent, &(list[1])), "Failed to detach in the middle.");
+    TEST_ASSERT_TRUE_MESSAGE(cJSON_DetachItemViaPointer(parent, &(list[0])) == &(list[0]), "Failed to detach in the middle.");
+}
+
 static void cjson_replace_item_via_pointer_should_replace_items(void)
 {
     cJSON replacements[3];
@@ -352,6 +384,19 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
 {
     char buffer[10];
     cJSON *item = cJSON_CreateString("item");
+    cJSON *array = cJSON_CreateArray();
+    cJSON *item1 = cJSON_CreateString("item1");
+    cJSON *item2 = cJSON_CreateString("corrupted array item3");
+    cJSON *corruptedString = cJSON_CreateString("corrupted");
+    struct cJSON *originalPrev;
+
+    add_item_to_array(array, item1);
+    add_item_to_array(array, item2);
+
+    originalPrev = item2->prev;
+    item2->prev = NULL;
+    free(corruptedString->valuestring);
+    corruptedString->valuestring = NULL;
 
     cJSON_InitHooks(NULL);
     TEST_ASSERT_NULL(cJSON_Parse(NULL));
@@ -411,6 +456,8 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
     cJSON_DeleteItemFromObject(item, NULL);
     cJSON_DeleteItemFromObjectCaseSensitive(NULL, "item");
     cJSON_DeleteItemFromObjectCaseSensitive(item, NULL);
+    TEST_ASSERT_FALSE(cJSON_InsertItemInArray(array, 0, NULL));
+    TEST_ASSERT_FALSE(cJSON_InsertItemInArray(array, 1, item));
     TEST_ASSERT_FALSE(cJSON_InsertItemInArray(NULL, 0, item));
     TEST_ASSERT_FALSE(cJSON_InsertItemInArray(item, 0, NULL));
     TEST_ASSERT_FALSE(cJSON_ReplaceItemViaPointer(NULL, item, item));
@@ -427,13 +474,38 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
     TEST_ASSERT_NULL(cJSON_Duplicate(NULL, true));
     TEST_ASSERT_FALSE(cJSON_Compare(item, NULL, false));
     TEST_ASSERT_FALSE(cJSON_Compare(NULL, item, false));
+    TEST_ASSERT_NULL(cJSON_SetValuestring(NULL, "test"));
+    TEST_ASSERT_NULL(cJSON_SetValuestring(corruptedString, "test"));
+    TEST_ASSERT_NULL(cJSON_SetValuestring(item, NULL));
     cJSON_Minify(NULL);
     /* skipped because it is only used via a macro that checks for NULL */
     /* cJSON_SetNumberHelper(NULL, 0); */
 
+    /* restore corrupted item2 to delete it */
+    item2->prev = originalPrev;
+    cJSON_Delete(corruptedString);
+    cJSON_Delete(array);
     cJSON_Delete(item);
 }
 
+static void cjson_set_valuestring_should_return_null_if_strings_overlap(void)
+{       
+    cJSON *obj;
+    char* str;
+    char* str2;
+
+    obj =  cJSON_Parse("\"foo0z\"");
+    
+    str =  cJSON_SetValuestring(obj, "abcde");
+    str += 1;
+    /* The string passed to strcpy overlap which is not allowed.*/
+    str2 = cJSON_SetValuestring(obj, str);
+    /* If it overlaps, the string will be messed up.*/
+    TEST_ASSERT_TRUE(strcmp(str, "bcde") == 0);
+    TEST_ASSERT_NULL(str2);
+    cJSON_Delete(obj);
+}
+
 static void *CJSON_CDECL failing_realloc(void *pointer, size_t size)
 {
     (void)size;
@@ -722,11 +794,14 @@ int CJSON_CDECL main(void)
     RUN_TEST(cjson_get_object_item_case_sensitive_should_not_crash_with_array);
     RUN_TEST(typecheck_functions_should_check_type);
     RUN_TEST(cjson_should_not_parse_to_deeply_nested_jsons);
+    RUN_TEST(cjson_should_not_follow_too_deep_circular_references);
     RUN_TEST(cjson_set_number_value_should_set_numbers);
     RUN_TEST(cjson_detach_item_via_pointer_should_detach_items);
+    RUN_TEST(cjson_detach_item_via_pointer_should_return_null_if_item_prev_is_null);
     RUN_TEST(cjson_replace_item_via_pointer_should_replace_items);
     RUN_TEST(cjson_replace_item_in_object_should_preserve_name);
     RUN_TEST(cjson_functions_should_not_crash_with_null_pointers);
+    RUN_TEST(cjson_set_valuestring_should_return_null_if_strings_overlap);
     RUN_TEST(ensure_should_fail_on_failed_realloc);
     RUN_TEST(skip_utf8_bom_should_skip_bom);
     RUN_TEST(skip_utf8_bom_should_not_skip_bom_if_not_at_beginning);

tests/parse_examples.c

@@ -250,6 +250,33 @@ static void test14_should_not_be_parsed(void)
     }
 }
 
+/* Address Sanitizer */
+static void test15_should_not_heap_buffer_overflow(void)
+{
+    const char *strings[] = {
+        "{\"1\":1,",
+        "{\"1\":1, ",
+    };
+
+    size_t i;
+
+    for (i = 0; i < sizeof(strings) / sizeof(strings[0]); i+=1)
+    {
+        const char *json_string = strings[i];
+        size_t len = strlen(json_string);
+        cJSON *json = NULL;
+
+        char *exact_size_heap = (char*)malloc(len);
+        TEST_ASSERT_NOT_NULL(exact_size_heap);
+
+        memcpy(exact_size_heap, json_string, len);
+        json = cJSON_ParseWithLength(exact_size_heap, len);
+
+        cJSON_Delete(json);
+        free(exact_size_heap);
+    }
+}
+
 int CJSON_CDECL main(void)
 {
     UNITY_BEGIN();
@@ -267,5 +294,6 @@ int CJSON_CDECL main(void)
     RUN_TEST(test12_should_not_be_parsed);
     RUN_TEST(test13_should_be_parsed_without_null_termination);
     RUN_TEST(test14_should_not_be_parsed);
+    RUN_TEST(test15_should_not_heap_buffer_overflow);
     return UNITY_END();
 }

tests/print_object.c

@@ -63,7 +63,7 @@ static void assert_print_object(const char * const expected, const char * const
 
     formatted_buffer.format = true;
     TEST_ASSERT_TRUE_MESSAGE(print_object(item, &formatted_buffer), "Failed to print formatted string.");
-    TEST_ASSERT_EQUAL_STRING_MESSAGE(expected, printed_formatted, "Formatted ojbect is not correct.");
+    TEST_ASSERT_EQUAL_STRING_MESSAGE(expected, printed_formatted, "Formatted object is not correct.");
 
     reset(item);
 }

tests/unity/auto/parse_output.rb

@@ -78,7 +78,7 @@ class ParseOutput
     @array_list.push '     <testcase classname="' + test_suite + '" name="' + test_name + '"/>'
   end
 
-  # Test was flagged as being ingored so format the output
+  # Test was flagged as being ignored so format the output
   def test_ignored(array)
     last_item = array.length - 1
     test_name = array[last_item - 2]

tests/unity/docs/UnityGettingStartedGuide.md

@@ -72,7 +72,7 @@ header files. These three files _are_ Unity.
 into this folder already. This is where all the handy documentation can be
 found.
 - `examples` - This contains a few examples of using Unity.
-- `extras` - These are optional add ons to Unity that are not part of the core
+- `extras` - These are optional addons to Unity that are not part of the core
 project. If you've reached us through James Grenning's book, you're going to
 want to look here.
 - `test` - This is how Unity and its scripts are all tested. If you're just using

tests/unity/extras/eclipse/error_parsers.txt

@@ -2,7 +2,7 @@ Eclipse error parsers
 =====================
 
 These are a godsend for extracting  & quickly navigating to
-warnings & error messages from console output. Unforunately
+warnings & error messages from console output. Unfortunately
 I don't know how to write an Eclipse plugin so you'll have
 to add them manually.
 

tests/unity/src/unity.c

@@ -8,7 +8,7 @@
 #include "unity.h"
 #include <stddef.h>
 
-/* If omitted from header, declare overrideable prototypes here so they're ready for use */
+/* If omitted from header, declare overridable prototypes here so they're ready for use */
 #ifdef UNITY_OMIT_OUTPUT_CHAR_HEADER_DECLARATION
 void UNITY_OUTPUT_CHAR(int);
 #endif

tests/unity/test/rakefile

@@ -26,7 +26,7 @@ task :prepare_for_tests => TEMP_DIRS
 
 include RakefileHelpers
 
-# Load proper GCC as defult configuration
+# Load proper GCC as default configuration
 DEFAULT_CONFIG_FILE = 'gcc_auto_stdint.yml'
 configure_toolchain(DEFAULT_CONFIG_FILE)