Skip to content

Fix use-after-free in overwrite_item for reference-type nodes#1047

Open
SongT-50 wants to merge 1 commit into
DaveGamble:masterfrom
SongT-50:fix/overwrite-item-reference-uaf
Open

Fix use-after-free in overwrite_item for reference-type nodes#1047
SongT-50 wants to merge 1 commit into
DaveGamble:masterfrom
SongT-50:fix/overwrite-item-reference-uaf

Conversation

@SongT-50

@SongT-50 SongT-50 commented Jul 3, 2026

Copy link
Copy Markdown

Summary

overwrite_item() (cJSON_Utils.c) frees root->string, root->valuestring, and root->child without checking the cJSON_StringIsConst / cJSON_IsReference flags that cJSON_Delete() already honors.

When a reference-type node (created via cJSON_CreateObjectReference, cJSON_CreateArrayReference, or cJSON_CreateStringReference) is used as the root object of a JSON-Patch root replacement/add (path "") through cJSONUtils_ApplyPatches, overwrite_item frees externally-owned memory that another object still references — a use-after-free / double-free.

This is distinct from #1010 (missing NULL-object check on the same path): there object is NULL; here object is a valid, non-NULL reference node, and the missing check is the reference flag, not a NULL guard. The two issues are independent.

Reproduction (AddressSanitizer)

cJSON *owner = cJSON_CreateObject();
cJSON_AddStringToObject(owner, "k", "v");        /* owner owns child "k" */
cJSON *ref = cJSON_CreateObjectReference(owner->child);  /* shared, not owned */
cJSON *patches = cJSON_Parse("[{\"op\":\"replace\",\"path\":\"\",\"value\":{\"new\":1}}]");
cJSONUtils_ApplyPatches(ref, patches);           /* overwrite_item frees owner's child */
cJSON_Delete(patches);
cJSON_Delete(ref);
cJSON_Delete(owner);                             /* use-after-free on the freed child */

ASAN reports heap-use-after-free:

  • freed in overwrite_item (cJSON_Utils.c) via cJSON_Delete(root->child) during apply_patchcJSONUtils_ApplyPatches
  • use in the later cJSON_Delete(owner) reading the already-freed child

With this patch applied, the same program runs clean (no ASAN report).

Fix

Mirror the exact guards cJSON_Delete() already applies, restoring internal consistency (no behavioral change for non-reference nodes):

if (!(root->type & cJSON_StringIsConst) && (root->string != NULL))
    cJSON_free(root->string);
if (!(root->type & cJSON_IsReference) && (root->valuestring != NULL))
    cJSON_free(root->valuestring);
if (!(root->type & cJSON_IsReference) && (root->child != NULL))
    cJSON_Delete(root->child);

All 22 existing tests pass (ctest), including json_patch_tests, old_utils_tests, and misc_utils_tests.

Note

Originally reported by email on April 6, 2026 (with a follow-up on June 17); opening as a PR after 90 days as the maintainer inbox appears inactive. Happy to adjust the approach to your preference.

Found via cross-model AI code review (independent analysis by Claude and Codex, using a generator/evaluator separation); the use-after-free was surfaced by the Codex pass.

overwrite_item() frees root->string, root->valuestring, and root->child
without checking the cJSON_StringIsConst / cJSON_IsReference flags that
cJSON_Delete() already honors. When a reference-type node (from
cJSON_CreateObjectReference / cJSON_CreateArrayReference /
cJSON_CreateStringReference) is used as the root object of a JSON-Patch
root replacement/add (path "") via cJSONUtils_ApplyPatches, overwrite_item
frees externally-owned memory that another object still references,
leading to use-after-free / double-free.

This mirrors the guards cJSON_Delete() applies, restoring internal
consistency. Distinct from DaveGamble#1010 (missing NULL-object check on the same
path): here object is a valid, non-NULL reference node.
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.

1 participant