Skip to content

cJSON_Utils: fix use-after-free in ApplyPatches when a patch removes the patch array - #1084

Open
iliasabk wants to merge 1 commit into
DaveGamble:masterfrom
iliasabk:fix/apply-patches-uaf
Open

iliasabk wants to merge 1 commit into
DaveGamble:masterfrom
iliasabk:fix/apply-patches-uaf

Conversation

@iliasabk

Copy link
Copy Markdown

Fixes #1082 — heap-use-after-free in cJSONUtils_ApplyPatches() / cJSONUtils_ApplyPatchesCaseSensitive() reported with full PoC + ASan trace (CWE-416).

Root cause

Both functions walk the patch array via current_patch->next. When the patch array is itself part of the document being patched and a remove operation deletes it, apply_patch()cJSON_Delete() frees the node the loop is iterating over; the next current_patch->next dereference reads freed memory.

cJSON *doc = cJSON_Parse("{\"patches\":[{\"op\":\"remove\",\"path\":\"/patches\"}]}");
cJSON *patches = cJSON_GetObjectItemCaseSensitive(doc, "patches");
cJSONUtils_ApplyPatches(doc, patches);  /* reads ->next from freed node */

ASan: heap-use-after-free READ of size 8 at cJSON_Utils.c:1061, freed by cJSON_Deleteapply_patch (cJSON_Utils.c:896).

This is the same class that #1065 fixed in cJSONUtils_MergePatch() — there the patch could be a subtree of the merge target, here the patch list can be a subtree of the patched object.

Fix

Iterate over a deep copy (cJSON_Duplicate(patches, true)) in both functions. apply_patch() already duplicates the value it inserts into object, so nothing in object points into the copy; deleting the copy after the loop is leak-free on both the success and error paths. The copy costs one duplicate per call — the alternative (documenting that patches must not be part of object) leaves the crash reachable.

Verification

  • Reproduced the exact reported trace under -fsanitize=address on master (6d9f244): heap-use-after-free READ at cJSON_Utils.c:1061apply_patchcJSON_Delete
  • Patched build: rc=0 for both cJSONUtils_ApplyPatches and cJSONUtils_ApplyPatchesCaseSensitive, zero ASan reports; normal patching (patches outside object) unaffected
  • Two new regression tests in tests/old_utils_tests.c (mirroring the Fix: heap-use-after-free in merge_patch when patch is subtree of target #1065 test pattern): they trigger the ASan abort on unfixed code (verified red), pass fixed
  • ctest: 22/22 pass under ASan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use-after-free in cJSONUtils_ApplyPatches when a patch removes the patch array

1 participant