Skip to content

Fix: heap-use-after-free in merge_patch when patch is subtree of target - #1065

Open
lilu5458 wants to merge 1 commit into
DaveGamble:masterfrom
lilu5458:fix/merge-patch-uaf-1060
Open

Fix: heap-use-after-free in merge_patch when patch is subtree of target#1065
lilu5458 wants to merge 1 commit into
DaveGamble:masterfrom
lilu5458:fix/merge-patch-uaf-1060

Conversation

@lilu5458

Copy link
Copy Markdown

Summary

Fixes a heap-use-after-free (UAF) in cJSONUtils_MergePatch() reported in #1060, where patch is a non-object subtree of target.

Root Cause

In merge_patch() (cJSON_Utils.c), when patch is a non-object (scalar, array, or NULL), the function used to:

cJSON_Delete(target);              // (A) frees target — and patch if it is a subtree
return cJSON_Duplicate(patch, 1); // (B) reads the already-freed patch memory → UAF

AddressSanitizer confirms the UAF read at cJSON_Duplicate_rec (cJSON.c:2808), freed by cJSON_Delete (cJSON.c:273) called from merge_patch (cJSON_Utils.c:1328).

Fix

Reorder the operations: duplicate patch into a local variable first, then delete target, then return the duplicate. This matches the "Option B — Duplicate before delete" approach proposed in #1060.

cJSON *duplicate = cJSON_Duplicate(patch, 1);
cJSON_Delete(target);
return duplicate;

This is a small bug/security fix, so it targets master per the contributing guide.

Verification

  • PoC: Reproduced the UAF with a minimal reproducer (poc_uaf_merge_patch.c) — target={"a":[1,2,3]}, patch=cJSON_GetObjectItem(target,"a"). Under ASan the original code crashes at cJSON_Duplicate_rec (cJSON.c:2808); after the fix the PoC runs cleanly (exit 0) and prints result: [1, 2, 3].
  • Regression test: Added merge_patch_should_not_read_freed_memory_when_patch_is_subtree to tests/old_utils_tests.c covering the subtree-patch scenario.
  • Full test suite: ctest passes 22/22 tests (including the new one) with no regressions.

Credit

The vulnerability and the "duplicate before delete" fix approach were independently reported in #1060 by @1820893135-pixel. This PR implements that fix, adds a PoC + ASan evidence, and adds a regression unit test.

Fixes #1060

When cJSONUtils_MergePatch(target, patch) is called with a non-object
patch (scalar, array, or NULL) that happens to be a subtree of target,
merge_patch() called cJSON_Delete(target) first, which freed the patch
memory, and then cJSON_Duplicate(patch, 1) read the already-freed memory,
triggering a heap-use-after-free (detected by AddressSanitizer at
cJSON_Duplicate_rec, cJSON.c:2808).

Fix: duplicate the patch first into a local variable, then delete the
target, then return the duplicate. This matches the Option B approach
proposed in issue DaveGamble#1060.

Verified locally:
- Reproduced the UAF with a minimal PoC under ASan before the fix.
- After the fix the PoC runs cleanly (exit 0, correct result [1,2,3]).
- Added a regression unit test
  (merge_patch_should_not_read_freed_memory_when_patch_is_subtree).
- Full ctest suite passes (22/22 tests).

Fixes DaveGamble#1060

Signed-off-by: lilu <lilu@kylinos.cn>
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.

Heap-Use-After-Free in cJSON merge_patch() via cJSONUtils_MergePatch()(Not found in existing CVE records or git history)

1 participant