add NULL checkings

Add NULL checkings in cJSON_InsertItemInArray and cJSON_SetValuestring Fixing #802(CVE-2023-50471) and #803(CVE-2023-50472)
DaveGamble · Dec 15, 2023 · 9759352 · 9759352
1 parent cb8693b
commit 9759352
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 2 deletions.
diff --git a/cJSON.c b/cJSON.c
@@ -401,7 +401,12 @@ CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring)
 {
     char *copy = NULL;
     /* 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;
+    }
+    /* return NULL if the object is corrupted */
+    if (object->valuestring == NULL)
     {
         return NULL;
     }
@@ -2264,7 +2269,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;
     }
@@ -2282,6 +2287,11 @@ CJSON_PUBLIC(cJSON_bool) cJSON_InsertItemInArray(cJSON *array, int which, cJSON
     {
         array->child = newitem;
     }
+    else if (newitem->prev == NULL)
+    {
+        /* return false if after_inserted is a corrupted array item */
+        return false;
+    }
     else
     {
         newitem->prev->next = newitem;

diff --git a/tests/misc_tests.c b/tests/misc_tests.c
@@ -352,6 +352,15 @@ 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 item");
+    cJSON *corruptedString = cJSON_CreateString("corrupted");
+
+    add_item_to_array(array, item1);
+    add_item_to_array(array, item2);
+    item2->prev = NULL;
+    corruptedString->valuestring = NULL;
 
     cJSON_InitHooks(NULL);
     TEST_ASSERT_NULL(cJSON_Parse(NULL));
@@ -411,6 +420,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,6 +438,8 @@ 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"));
     cJSON_Minify(NULL);
     /* skipped because it is only used via a macro that checks for NULL */
     /* cJSON_SetNumberHelper(NULL, 0); */