From d91a728ae0522b5772cc904f41fd9a8d49ce2e7d Mon Sep 17 00:00:00 2001 From: Mobin Dariush <34644374+dalisyron@users.noreply.github.com> Date: Mon, 8 Jun 2026 23:12:28 +0200 Subject: [PATCH] fix: prevent NULL valuestring dereference in cJSONUtils_ApplyPatches cJSON_IsString() only checks the type tag, so a cJSON_String item whose valuestring is NULL (e.g. one created with cJSON_CreateStringReference(NULL)) passes the check. The JSON patch code then dereferences the "op", "path" and "from" valuestrings via strcmp()/indexing/strdup(), causing a NULL pointer dereference instead of rejecting the malformed patch. Reject a NULL valuestring alongside the existing cJSON_IsString() checks so these patches return the usual malformed-patch error codes, matching how other invalid patches are handled. Fixes #1011 --- cJSON_Utils.c | 6 ++--- tests/misc_utils_tests.c | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 8b38eb253..546610233 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -742,7 +742,7 @@ enum patch_operation { INVALID, ADD, REMOVE, REPLACE, MOVE, COPY, TEST }; static enum patch_operation decode_patch_operation(const cJSON * const patch, const cJSON_bool case_sensitive) { cJSON *operation = get_object_item(patch, "op", case_sensitive); - if (!cJSON_IsString(operation)) + if (!cJSON_IsString(operation) || (operation->valuestring == NULL)) { return INVALID; } @@ -815,7 +815,7 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_ int status = 0; path = get_object_item(patch, "path", case_sensitive); - if (!cJSON_IsString(path)) + if (!cJSON_IsString(path) || (path->valuestring == NULL)) { /* malformed patch. */ status = 2; @@ -906,7 +906,7 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_ if ((opcode == MOVE) || (opcode == COPY)) { cJSON *from = get_object_item(patch, "from", case_sensitive); - if (!cJSON_IsString(from)) + if (!cJSON_IsString(from) || (from->valuestring == NULL)) { /* missing "from" for copy/move. */ status = 4; diff --git a/tests/misc_utils_tests.c b/tests/misc_utils_tests.c index 7d300bc8e..c5bd9ccad 100644 --- a/tests/misc_utils_tests.c +++ b/tests/misc_utils_tests.c @@ -70,11 +70,66 @@ static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static void cjson_utils_apply_patches_should_reject_null_valuestring(void) +{ + /* A cJSON_String item can have a NULL valuestring (e.g. when created with + * cJSON_CreateStringReference(NULL)). The JSON patch code checks the "op", + * "path" and "from" fields with cJSON_IsString() and then dereferences + * valuestring. Such patches must be rejected with an error code instead of + * crashing on the NULL valuestring. */ + + /* NULL "op" valuestring */ + { + cJSON *object = cJSON_CreateObject(); + cJSON *patches = cJSON_CreateArray(); + cJSON *patch = cJSON_CreateObject(); + cJSON_AddItemToObject(patch, "op", cJSON_CreateStringReference(NULL)); + cJSON_AddItemToObject(patch, "path", cJSON_CreateString("")); + cJSON_AddItemToObject(patch, "value", cJSON_CreateNumber(1)); + cJSON_AddItemToArray(patches, patch); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatches(object, patches), "NULL \"op\" valuestring should be rejected."); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatchesCaseSensitive(object, patches), "NULL \"op\" valuestring should be rejected."); + cJSON_Delete(object); + cJSON_Delete(patches); + } + + /* NULL "path" valuestring */ + { + cJSON *object = cJSON_CreateObject(); + cJSON *patches = cJSON_CreateArray(); + cJSON *patch = cJSON_CreateObject(); + cJSON_AddItemToObject(patch, "op", cJSON_CreateString("remove")); + cJSON_AddItemToObject(patch, "path", cJSON_CreateStringReference(NULL)); + cJSON_AddItemToArray(patches, patch); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatches(object, patches), "NULL \"path\" valuestring should be rejected."); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatchesCaseSensitive(object, patches), "NULL \"path\" valuestring should be rejected."); + cJSON_Delete(object); + cJSON_Delete(patches); + } + + /* NULL "from" valuestring */ + { + cJSON *object = cJSON_CreateObject(); + cJSON *patches = cJSON_CreateArray(); + cJSON *patch = cJSON_CreateObject(); + cJSON_AddItemToObject(object, "a", cJSON_CreateNumber(1)); + cJSON_AddItemToObject(patch, "op", cJSON_CreateString("move")); + cJSON_AddItemToObject(patch, "path", cJSON_CreateString("/b")); + cJSON_AddItemToObject(patch, "from", cJSON_CreateStringReference(NULL)); + cJSON_AddItemToArray(patches, patch); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatches(object, patches), "NULL \"from\" valuestring should be rejected."); + TEST_ASSERT_TRUE_MESSAGE(0 != cJSONUtils_ApplyPatchesCaseSensitive(object, patches), "NULL \"from\" valuestring should be rejected."); + cJSON_Delete(object); + cJSON_Delete(patches); + } +} + int main(void) { UNITY_BEGIN(); RUN_TEST(cjson_utils_functions_shouldnt_crash_with_null_pointers); + RUN_TEST(cjson_utils_apply_patches_should_reject_null_valuestring); return UNITY_END(); }