diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 4f3a9ed8d..84ae6fbfe 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -1035,9 +1035,17 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_ return status; } -CJSON_PUBLIC(int) cJSONUtils_ApplyPatches(cJSON * const object, const cJSON * const patches) +/* Apply a patch array atomically: run every operation against a copy + * and only commit to the caller's document when all of them succeed. + * A failing patch must not leave earlier operations' destruction + * behind (CVE-2026-67217). Working from copies also keeps the loop + * safe when the patch array itself lives inside the patched document + * (applying a patch may delete it). */ +static int apply_patches(cJSON * const object, const cJSON * const patches, const cJSON_bool case_sensitive) { const cJSON *current_patch = NULL; + cJSON *patches_copy = NULL; + cJSON *work = NULL; int status = 0; if (!cJSON_IsArray(patches)) @@ -1046,51 +1054,50 @@ CJSON_PUBLIC(int) cJSONUtils_ApplyPatches(cJSON * const object, const cJSON * co return 1; } - if (patches != NULL) + /* Duplicate first: either input may alias the other, and applying a + * patch can delete nodes out from under the iteration. */ + patches_copy = cJSON_Duplicate(patches, 1); + work = cJSON_Duplicate(object, 1); + if ((patches_copy == NULL) || (work == NULL)) { - current_patch = patches->child; + /* out of memory (or NULL document); nothing was modified. */ + cJSON_Delete(patches_copy); + cJSON_Delete(work); + return 1; } + current_patch = patches_copy->child; while (current_patch != NULL) { - status = apply_patch(object, current_patch, false); + status = apply_patch(work, current_patch, case_sensitive); if (status != 0) { + cJSON_Delete(patches_copy); + cJSON_Delete(work); return status; } current_patch = current_patch->next; } + /* All operations succeeded: commit the copy over the original, + * keeping the caller's pointer stable. */ + work->next = object->next; + work->prev = object->prev; + overwrite_item(object, *work); + cJSON_free(work); + cJSON_Delete(patches_copy); + return 0; } -CJSON_PUBLIC(int) cJSONUtils_ApplyPatchesCaseSensitive(cJSON * const object, const cJSON * const patches) +CJSON_PUBLIC(int) cJSONUtils_ApplyPatches(cJSON * const object, const cJSON * const patches) { - const cJSON *current_patch = NULL; - int status = 0; - - if (!cJSON_IsArray(patches)) - { - /* malformed patches. */ - return 1; - } - - if (patches != NULL) - { - current_patch = patches->child; - } - - while (current_patch != NULL) - { - status = apply_patch(object, current_patch, true); - if (status != 0) - { - return status; - } - current_patch = current_patch->next; - } + return apply_patches(object, patches, false); +} - return 0; +CJSON_PUBLIC(int) cJSONUtils_ApplyPatchesCaseSensitive(cJSON * const object, const cJSON * const patches) +{ + return apply_patches(object, patches, true); } static void compose_patch(cJSON * const patches, const unsigned char * const operation, const unsigned char * const path, const unsigned char *suffix, const cJSON * const value) diff --git a/cJSON_Utils.h b/cJSON_Utils.h index a970c6504..b378f73c2 100644 --- a/cJSON_Utils.h +++ b/cJSON_Utils.h @@ -44,26 +44,9 @@ CJSON_PUBLIC(void) cJSONUtils_AddPatchToArray(cJSON * const array, const char * CJSON_PUBLIC(int) cJSONUtils_ApplyPatches(cJSON * const object, const cJSON * const patches); CJSON_PUBLIC(int) cJSONUtils_ApplyPatchesCaseSensitive(cJSON * const object, const cJSON * const patches); -/* -// Note that ApplyPatches is NOT atomic on failure. To implement an atomic ApplyPatches, use: -//int cJSONUtils_AtomicApplyPatches(cJSON **object, cJSON *patches) -//{ -// cJSON *modme = cJSON_Duplicate(*object, 1); -// int error = cJSONUtils_ApplyPatches(modme, patches); -// if (!error) -// { -// cJSON_Delete(*object); -// *object = modme; -// } -// else -// { -// cJSON_Delete(modme); -// } -// -// return error; -//} -// Code not added to library since this strategy is a LOT slower. -*/ +/* cJSONUtils_ApplyPatches and cJSONUtils_ApplyPatchesCaseSensitive are + * atomic: either all of the patches are applied, or the object is left + * unchanged. */ /* Implement RFC7386 (https://tools.ietf.org/html/rfc7396) JSON Merge Patch spec. */ /* target will be modified by patch. return value is new ptr for target. */ diff --git a/tests/misc_utils_tests.c b/tests/misc_utils_tests.c index 7d300bc8e..a3895d6fd 100644 --- a/tests/misc_utils_tests.c +++ b/tests/misc_utils_tests.c @@ -70,11 +70,122 @@ static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static void apply_patches_should_leave_document_untouched_when_a_patch_fails(void) +{ + /* CVE-2026-67217: patch arrays must apply atomically. A failing + * operation must not leave earlier operations applied. */ + cJSON *doc = cJSON_Parse("{\"a\": 1, \"b\": 2}"); + cJSON *patch = cJSON_Parse("[{\"op\": \"remove\", \"path\": \"/a\"}, " + "{\"op\": \"remove\", " + "\"path\": \"/nonexistent\"}]"); + cJSON *a = NULL; + cJSON *b = NULL; + + TEST_ASSERT_NOT_NULL(doc); + TEST_ASSERT_NOT_NULL(patch); + + TEST_ASSERT_NOT_EQUAL(0, cJSONUtils_ApplyPatches(doc, patch)); + + a = cJSON_GetObjectItem(doc, "a"); + b = cJSON_GetObjectItem(doc, "b"); + TEST_ASSERT_NOT_NULL(a); + TEST_ASSERT_NOT_NULL(b); + TEST_ASSERT_EQUAL_INT(1, a->valueint); + TEST_ASSERT_EQUAL_INT(2, b->valueint); + + cJSON_Delete(doc); + cJSON_Delete(patch); +} + +static void apply_patches_should_apply_valid_patches(void) +{ + cJSON *doc = cJSON_Parse("{\"a\": 1}"); + cJSON *patch = cJSON_Parse("[{\"op\": \"replace\", \"path\": \"/a\", " + "\"value\": 2}, " + "{\"op\": \"add\", \"path\": \"/c\", " + "\"value\": 3}]"); + cJSON *a = NULL; + cJSON *c = NULL; + + TEST_ASSERT_NOT_NULL(doc); + TEST_ASSERT_NOT_NULL(patch); + + TEST_ASSERT_EQUAL_INT(0, cJSONUtils_ApplyPatches(doc, patch)); + + a = cJSON_GetObjectItem(doc, "a"); + c = cJSON_GetObjectItem(doc, "c"); + TEST_ASSERT_NOT_NULL(a); + TEST_ASSERT_NOT_NULL(c); + TEST_ASSERT_EQUAL_INT(2, a->valueint); + TEST_ASSERT_EQUAL_INT(3, c->valueint); + + cJSON_Delete(doc); + cJSON_Delete(patch); +} + +static void apply_patches_should_preserve_non_root_links(const cJSON_bool array, const cJSON_bool case_sensitive) +{ + int index = 0; + for (index = 0; index < 3; index++) + { + cJSON *doc = cJSON_Parse(array ? "[{\"v\":1},{\"v\":1},{\"v\":1}]" : + "{\"first\":{\"v\":1},\"middle\":{\"v\":1},\"last\":{\"v\":1}}"); + cJSON *patch = cJSON_Parse("[{\"op\":\"replace\",\"path\":\"/v\",\"value\":2}]"); + cJSON *target = NULL; + cJSON *previous = NULL; + cJSON *next = NULL; + + TEST_ASSERT_NOT_NULL(doc); + TEST_ASSERT_NOT_NULL(patch); + target = cJSON_GetArrayItem(doc, index); + TEST_ASSERT_NOT_NULL(target); + previous = target->prev; + next = target->next; + + TEST_ASSERT_EQUAL_INT(0, case_sensitive ? cJSONUtils_ApplyPatchesCaseSensitive(target, patch) : + cJSONUtils_ApplyPatches(target, patch)); + TEST_ASSERT_EQUAL_PTR(previous, target->prev); + TEST_ASSERT_EQUAL_PTR(next, target->next); + TEST_ASSERT_EQUAL_PTR(target, cJSON_GetArrayItem(doc, index)); + TEST_ASSERT_EQUAL_PTR(index == 0 ? target : previous->next, target); + if (next != NULL) + { + TEST_ASSERT_EQUAL_PTR(target, next->prev); + } + TEST_ASSERT_EQUAL_PTR(cJSON_GetArrayItem(doc, 2), doc->child->prev); + TEST_ASSERT_EQUAL_INT(3, cJSON_GetArraySize(doc)); + TEST_ASSERT_EQUAL_INT(2, cJSON_GetObjectItem(target, "v")->valueint); + if (!array) + { + TEST_ASSERT_EQUAL_STRING(index == 0 ? "first" : (index == 1 ? "middle" : "last"), target->string); + TEST_ASSERT_EQUAL_PTR(target, cJSON_GetObjectItem(doc, target->string)); + } + cJSON_Delete(patch); + cJSON_Delete(doc); + } +} + +static void apply_patches_should_preserve_object_siblings(void) +{ + apply_patches_should_preserve_non_root_links(false, false); + apply_patches_should_preserve_non_root_links(false, true); +} + +static void apply_patches_should_preserve_array_siblings(void) +{ + apply_patches_should_preserve_non_root_links(true, false); + apply_patches_should_preserve_non_root_links(true, true); +} + int main(void) { UNITY_BEGIN(); RUN_TEST(cjson_utils_functions_shouldnt_crash_with_null_pointers); + RUN_TEST(apply_patches_should_leave_document_untouched_when_a_patch_fails); + RUN_TEST(apply_patches_should_apply_valid_patches); + RUN_TEST(apply_patches_should_preserve_object_siblings); + RUN_TEST(apply_patches_should_preserve_array_siblings); return UNITY_END(); }