Skip to content

Commit 60ff122

Browse files
add NULL checkings (DaveGamble#809)
* add NULL checks in cJSON_SetValuestring Fixes DaveGamble#803(CVE-2023-50472) * add NULL check in cJSON_InsertItemInArray Fixes DaveGamble#802(CVE-2023-50471) * add tests for NULL checks add tests for NULL checks in cJSON_InsertItemInArray and cJSON_SetValuestring
1 parent cb8693b commit 60ff122

File tree

2 files changed

+33
-2
lines changed

2 files changed

+33
-2
lines changed

cJSON.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,12 @@ CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring)
401401
{
402402
char *copy = NULL;
403403
/* if object's type is not cJSON_String or is cJSON_IsReference, it should not set valuestring */
404-
if (!(object->type & cJSON_String) || (object->type & cJSON_IsReference))
404+
if ((object == NULL) || !(object->type & cJSON_String) || (object->type & cJSON_IsReference))
405+
{
406+
return NULL;
407+
}
408+
/* return NULL if the object is corrupted */
409+
if (object->valuestring == NULL)
405410
{
406411
return NULL;
407412
}
@@ -2264,7 +2269,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_InsertItemInArray(cJSON *array, int which, cJSON
22642269
{
22652270
cJSON *after_inserted = NULL;
22662271

2267-
if (which < 0)
2272+
if (which < 0 || newitem == NULL)
22682273
{
22692274
return false;
22702275
}
@@ -2275,6 +2280,11 @@ CJSON_PUBLIC(cJSON_bool) cJSON_InsertItemInArray(cJSON *array, int which, cJSON
22752280
return add_item_to_array(array, newitem);
22762281
}
22772282

2283+
if (after_inserted != array->child && newitem->prev == NULL) {
2284+
/* return false if after_inserted is a corrupted array item */
2285+
return false;
2286+
}
2287+
22782288
newitem->next = after_inserted;
22792289
newitem->prev = after_inserted->prev;
22802290
after_inserted->prev = newitem;

tests/misc_tests.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,19 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
352352
{
353353
char buffer[10];
354354
cJSON *item = cJSON_CreateString("item");
355+
cJSON *array = cJSON_CreateArray();
356+
cJSON *item1 = cJSON_CreateString("item1");
357+
cJSON *item2 = cJSON_CreateString("corrupted array item3");
358+
cJSON *corruptedString = cJSON_CreateString("corrupted");
359+
struct cJSON *originalPrev;
360+
361+
add_item_to_array(array, item1);
362+
add_item_to_array(array, item2);
363+
364+
originalPrev = item2->prev;
365+
item2->prev = NULL;
366+
free(corruptedString->valuestring);
367+
corruptedString->valuestring = NULL;
355368

356369
cJSON_InitHooks(NULL);
357370
TEST_ASSERT_NULL(cJSON_Parse(NULL));
@@ -411,6 +424,8 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
411424
cJSON_DeleteItemFromObject(item, NULL);
412425
cJSON_DeleteItemFromObjectCaseSensitive(NULL, "item");
413426
cJSON_DeleteItemFromObjectCaseSensitive(item, NULL);
427+
TEST_ASSERT_FALSE(cJSON_InsertItemInArray(array, 0, NULL));
428+
TEST_ASSERT_FALSE(cJSON_InsertItemInArray(array, 1, item));
414429
TEST_ASSERT_FALSE(cJSON_InsertItemInArray(NULL, 0, item));
415430
TEST_ASSERT_FALSE(cJSON_InsertItemInArray(item, 0, NULL));
416431
TEST_ASSERT_FALSE(cJSON_ReplaceItemViaPointer(NULL, item, item));
@@ -427,10 +442,16 @@ static void cjson_functions_should_not_crash_with_null_pointers(void)
427442
TEST_ASSERT_NULL(cJSON_Duplicate(NULL, true));
428443
TEST_ASSERT_FALSE(cJSON_Compare(item, NULL, false));
429444
TEST_ASSERT_FALSE(cJSON_Compare(NULL, item, false));
445+
TEST_ASSERT_NULL(cJSON_SetValuestring(NULL, "test"));
446+
TEST_ASSERT_NULL(cJSON_SetValuestring(corruptedString, "test"));
430447
cJSON_Minify(NULL);
431448
/* skipped because it is only used via a macro that checks for NULL */
432449
/* cJSON_SetNumberHelper(NULL, 0); */
433450

451+
/* restore corrupted item2 to delete it */
452+
item2->prev = originalPrev;
453+
cJSON_Delete(corruptedString);
454+
cJSON_Delete(array);
434455
cJSON_Delete(item);
435456
}
436457

0 commit comments

Comments
 (0)