DaveGamble/cJSON
1
0
Fork 0
mirror of https://github.com/DaveGamble/cJSON.git synced 2026-09-24 23:07:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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 #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 #1060

Signed-off-by: lilu <[email protected]>
This commit is contained in:
lilu5458
2026-09-16 09:55:35 +08:00
committed by GitHub
parent fb16e5cf35
commit 6d9f2443ab
2 changed files with 40 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cJSON_Utils.c
+6 -2
View File
@@ -1324,9 +1324,13 @@ static cJSON *merge_patch(cJSON *target, const cJSON * const patch, const cJSON_
if (!cJSON_IsObject(patch)) if (!cJSON_IsObject(patch))
{ {
/* scalar value, array or NULL, just duplicate */ /* scalar value, array or NULL, just duplicate.
* Duplicate the patch first in case it is a subtree of target,
* otherwise cJSON_Delete(target) would free the patch memory
* and the subsequent cJSON_Duplicate would read freed memory. */
cJSON *duplicate = cJSON_Duplicate(patch, 1);
cJSON_Delete(target); cJSON_Delete(target);
return cJSON_Duplicate(patch, 1); return duplicate;
} }
if (!cJSON_IsObject(target)) if (!cJSON_IsObject(target))
tests/old_utils_tests.c
+34
View File
@@ -189,6 +189,39 @@ static void merge_tests(void)
} }
} }
static void merge_patch_should_not_read_freed_memory_when_patch_is_subtree(void)
{
/* When patch is a subtree of target, merge_patch must duplicate the patch
* before deleting target. Otherwise cJSON_Delete(target) frees the patch
* memory and the subsequent cJSON_Duplicate reads freed memory (UAF).
* See CVE candidate: heap-use-after-free in merge_patch (cJSON_Utils.c). */
cJSON *target = cJSON_Parse("{\"a\":[1,2,3]}");
cJSON *patch = cJSON_GetObjectItem(target, "a");
cJSON *result = NULL;
cJSON *first = NULL;
cJSON *second = NULL;
cJSON *third = NULL;
TEST_ASSERT_NOT_NULL(target);
TEST_ASSERT_NOT_NULL(patch);
/* patch (array [1,2,3]) is a subtree of target. This used to trigger
* heap-use-after-free under AddressSanitizer before the fix. */
result = cJSONUtils_MergePatch(target, patch);
TEST_ASSERT_NOT_NULL(result);
TEST_ASSERT_TRUE(cJSON_IsArray(result));
TEST_ASSERT_EQUAL_INT(3, cJSON_GetArraySize(result));
first = cJSON_GetArrayItem(result, 0);
second = cJSON_GetArrayItem(result, 1);
third = cJSON_GetArrayItem(result, 2);
TEST_ASSERT_EQUAL_INT(1, first->valueint);
TEST_ASSERT_EQUAL_INT(2, second->valueint);
TEST_ASSERT_EQUAL_INT(3, third->valueint);
cJSON_Delete(result);
}
static void generate_merge_tests(void) static void generate_merge_tests(void)
{ {
size_t i = 0; size_t i = 0;
@@ -219,6 +252,7 @@ int main(void)
RUN_TEST(misc_tests); RUN_TEST(misc_tests);
RUN_TEST(sort_tests); RUN_TEST(sort_tests);
RUN_TEST(merge_tests); RUN_TEST(merge_tests);
RUN_TEST(merge_patch_should_not_read_freed_memory_when_patch_is_subtree);
RUN_TEST(generate_merge_tests); RUN_TEST(generate_merge_tests);
return UNITY_END(); return UNITY_END();