Repository navigation
Conversation
…and contains patch
When cJSONUtils_MergePatch(target, patch) is called with an object
patch that is a subtree of a non-object target (for example, target
is the array [{"a":1}] and patch is its element), merge_patch() called
cJSON_Delete(target), which also freed patch, and then read
patch->child, triggering a heap-use-after-free.
This is the same situation as DaveGamble#1060, fixed in 6d9f244 for non-object
patches; that fix did not cover the branch where the patch is an
object and the target is not.
Fix: in that branch, duplicate the patch and create the new object
before deleting target, then merge from the copy into the new object
and free the copy. On allocation failure, free both and return NULL
without touching target.
Reproducer:
cJSON *target = cJSON_Parse("[{\"a\":1}]");
cJSON *patch = target->child;
cJSON *out = cJSONUtils_MergePatch(target, patch);
|
Independent validation: I checked out this PR head and ran an ASan reproducer for the exact variant (MergePatch with an object patch that is a subtree of an array target) -- ASan-clean, correct result {"a":1}, and the full ctest suite is 19/19 green. The duplicate-before-delete approach matches the merged #1065 pattern. Looks correct. One suggestion: this has no regression test, so the variant could silently regress. The test from the #1065 follow-up pattern would be (tests/old_utils_tests.c): cJSON *target = cJSON_Parse("[1, 2, {"a": 1}]"); Happy to submit that as a companion PR if the author prefers. |
|
@w4ffl35 I'm happy with that too, thank you |
…026-29036) decode_pointer_inplace advanced the write pointer on every loop iteration but never copied passthrough bytes down, so any escape not at end of token truncated the decoded name ('/a~1b' became 'a~'). Worse, the old code wrote '/' to decoded[1] instead of decoded[0] for ~1. JSON Patch add/remove/replace then targeted wrong keys or failed, enabling silent data corruption and wrong-key deletion via crafted ~0/~1 sequences. Rewrite compacts properly: escapes collapse two bytes into one, all other bytes copied down; decoded never overtakes the read cursor. Invalid escapes still abort as before. Regression cases 16/17 in cjson-utils-tests.json. Red/green: new cases fail before (apply error 13), all 3 patch suites pass after; full ctest 21/22 (old_utils_tests fails only on the unrelated not-yet-merged DaveGamble#1109 variant, identical before/after this change).
When
cJSONUtils_MergePatch(target, patch)is called with an object patch that is a subtree of a non-object target (for example, target is the array[{"a":1}]and patch is its element),merge_patch()calledcJSON_Delete(target), which also freed patch, and then readpatch->child, triggering a heap-use-after-free.This is the same situation as #1060, fixed in 6d9f244 for non-object patches; that fix did not cover the branch where the patch is an object and the target is not.
Fix: in that branch, duplicate the patch and create the new object before deleting target, then merge from the copy into the new object and free the copy. On allocation failure, free both and return NULL without touching target.
Reproducer: