Repository navigation
cJSON_Utils: survive allocation failures in the patch/merge/pointer helpers - #1102
Open
bg0d-droid wants to merge 1 commit into
Open
bg0d-droid wants to merge 1 commit into
bg0d-droid wants to merge 1 commit into
Conversation
…elpers The path-building helpers in cJSON_Utils.c used the result of cJSON_malloc without checking it and then wrote into it with sprintf/strcat, so any allocation failure crashed the process with a NULL-pointer write. This affects compose_patch(), create_patches() and cJSONUtils_FindPointerFromObjectTo(). Several sites also kept or dropped ownership of a value depending on an unchecked cJSON_AddItemToObject()/cJSON_AddItemToArray() call. Those functions leave the item caller-owned when they fail (for example when duplicating the key runs out of memory), so the item leaked: in merge_patch() the detached subtree, in apply_patch() the duplicated patch value (value was cleared unconditionally), in generate_merge_patch() the created/duplicated/recursive patch entries, and in compose_patch() the created op/path/value strings. Fix both classes systematically in the affected functions: check every allocation and only transfer ownership when the add actually succeeded, otherwise release the orphaned value and fail cleanly (NULL return, or the existing status-code convention in apply_patch()). Found with allocation-failure injection over cJSONUtils_GeneratePatches/ ApplyPatches/MergePatch/GenerateMergePatch/FindPointerFromObjectTo: 41 injection points crashed or leaked (469 in the wider sweep across parse, print and object APIs before triage). Regression coverage: tests/utils_oom_tests.c routes all cJSON allocations through cJSON_InitHooks and injects a failure at every step of the patch, merge and pointer operations, asserting that nothing remains allocated after each attempt. It segfaults against the unpatched code and passes with this fix; the rest of the suite (23 tests) still passes.
bg0d-droid
force-pushed
the
fix-utils-oom-robustness
branch
from
October 15, 2026 07:59
9a9cea0 to
287279e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two failure classes in
cJSON_Utils.cturn a simple allocation failure into a crash or a leak:NULL-pointer writes (crash). The path-building helpers use the result of
cJSON_mallocwithout checking it and then write into it withsprintf/strcat. Any allocation failure segfaults the process. Affected:compose_patch(),create_patches()(array and object branches) andcJSONUtils_FindPointerFromObjectTo()(array and object branches).Leaked values on failed insertion.
cJSON_AddItemToObject()/cJSON_AddItemToArray()leave the item caller-owned when they fail (e.g. when duplicating the key runs out of memory), but several call sites ignored the result and dropped or never took ownership of the value:merge_patch()— the detachedreplacementsubtree (and the recreatedtargetobject could be NULL and was used unchecked)apply_patch()— the duplicated patchvalue(value = NULLwas set unconditionally after the add)generate_merge_patch()— the creatednull/ duplicated / recursively generated patch entriescompose_patch()— the createdop/path/valuestringsChanges
All in
cJSON_Utils.c: check everycJSON_mallocresult and fail cleanly; only transfer ownership of a value when the add actually succeeded, releasing the orphaned value otherwise (NULL return from the generators, or the existing status-code convention withgoto cleanupinapply_patch()).tests/utils_oom_tests.c(plus one line intests/CMakeLists.txt) is a regression test that routes all cJSON allocations throughcJSON_InitHooksand injects a failure at every allocation step of the patch, merge, and pointer operations, asserting after each attempt that nothing remains allocated.Reproduction (unpatched)
The regression test segfaults against the unpatched code:
Found with allocation-failure injection: a sweep that fails each allocation of a call in turn produced 41 crashing or leaking injection points across
cJSONUtils_GeneratePatches/ApplyPatches/MergePatch/GenerateMergePatch/FindPointerFromObjectTo(LeakSanitizer traces show the leaked subtree inmerge_patch/apply_patch/generate_merge_patch, and the NULL-write stacks point intosprintfon the uncheckedcJSON_mallocresults).Testing
tests/utils_oom_testspasses after this fix (5 cases × 120 injection points each) and segfaults before it.ctest).