fix: prevent NULL valuestring dereference in cJSONUtils_ApplyPatches#1033
Open
dalisyron wants to merge 1 commit into
Open
fix: prevent NULL valuestring dereference in cJSONUtils_ApplyPatches#1033dalisyron wants to merge 1 commit into
dalisyron wants to merge 1 commit into
Conversation
cJSON_IsString() only checks the type tag, so a cJSON_String item whose valuestring is NULL (e.g. one created with cJSON_CreateStringReference(NULL)) passes the check. The JSON patch code then dereferences the "op", "path" and "from" valuestrings via strcmp()/indexing/strdup(), causing a NULL pointer dereference instead of rejecting the malformed patch. Reject a NULL valuestring alongside the existing cJSON_IsString() checks so these patches return the usual malformed-patch error codes, matching how other invalid patches are handled. Fixes DaveGamble#1011
dalisyron
force-pushed
the
fix/patch-null-valuestring
branch
from
02d632d to
d91a728
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
1 participant
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.
Description
cJSONUtils_ApplyPatches/...CaseSensitivecan crash with a NULL pointer dereference when a patch contains acJSON_Stringfield whosevaluestringisNULL, instead of rejecting the malformed patch.Root cause
The patch code validates the
op,pathandfromfields withcJSON_IsString(), which only checks the type tag ((type & 0xFF) == cJSON_String) and does not guaranteevaluestring != NULL. A string item with a NULLvaluestringis reachable through the public API (e.g.cJSON_CreateStringReference(NULL)), so it passes the check and is then dereferenced:decode_patch_operation()→strcmp(operation->valuestring, ...)apply_patch()→path->valuestring[0]/detach_path(..., path->valuestring)apply_patch()→detach_path()/get_item_from_pointer(..., from->valuestring)This continues the hardening from #1006, which tightened the same
fromcheck from== NULLto!cJSON_IsString(from).Fix
Reject a NULL
valuestringalongside the existingcJSON_IsString()checks forop,pathandfrom, so the affected patches return the existing malformed-patch error codes (3/2/4) instead of crashing — consistent with how other invalid patches are already handled.Verification
cjson_utils_apply_patches_should_reject_null_valuestringintests/misc_utils_tests.ccovering all three fields (the three PoCs from the issue). It crashes (NULL deref) onmasterand passes with this change.ENABLE_SANITIZERS,ENABLE_VALGRIND, none}: 22/22 tests pass, 0 warnings under-std=c89 -pedantic -Werror -Wconversion, and valgrind reports no leaks/errors.Fixes #1011