Skip to content

[Bug] Null Pointer Dereference in compose_patch() when cJSON_malloc fails #1040

Description

@BoxStrikesTeam

Summary

compose_patch() in cJSON_Utils.c dereferences the return value of cJSON_malloc() without first checking whether the allocation succeeded. If cJSON_malloc returns NULL (e.g. under memory pressure), the subsequent sprintf and encode_string_as_pointer calls write through a null pointer, causing undefined behaviour / segmentation fault.

Affected Version

cJSON v1.7.19 (latest release)

Affected File & Lines

File: cJSON_Utils.c
Function: compose_patch() (static)
Lines: 1120–1123

// cJSON_Utils.c — compose_patch(), lines 1120-1123
unsigned char *full_path = (unsigned char*)cJSON_malloc(path_length + suffix_length + sizeof("/"));

sprintf((char*)full_path, "%s/", (const char*)path);          // ← NPD if full_path == NULL
encode_string_as_pointer(full_path + path_length + 1, suffix); // ← NPD if full_path == NULL

Root Cause

When the suffix parameter is non-NULL (the else branch at line 1116), compose_patch allocates full_path via cJSON_malloc. However, there is no NULL check between the allocation and its first use. If the allocator returns NULL, the code immediately writes to address 0 + offset, causing a null pointer dereference.

This is a classic "missing NULL check after malloc" pattern.

Call Chain

compose_patch is called from multiple places, most notably:

cJSONUtils_GeneratePatches()
  └─ create_patches()
       ├─ compose_patch(..., "replace", path, NULL, to)     ← safe (suffix==NULL)
       ├─ compose_patch(..., "remove", path, new_path, NULL) ← VULNERABLE (suffix!=NULL)
       └─ compose_patch(..., "add",    path, "-", to_child)  ← VULNERABLE (suffix!=NULL)

Any path through create_patches that produces a "remove", "add", or "move" patch with a non-NULL suffix can trigger this bug.

Minimal Reproducer

#include <stdlib.h>
#include "cJSON.h"
#include "cJSON_Utils.h"

/* Force the next malloc call to fail */
static int fail_next = 0;
static void *failing_malloc(size_t size) {
    if (fail_next--) return NULL;
    return malloc(size);
}
static void failing_free(void *ptr) { free(ptr); }

int main(void) {
    cJSON_Hooks hooks = { failing_malloc, failing_free };
    cJSON_InitHooks(&hooks);

    cJSON *from = cJSON_Parse("{\"a\":1}");
    cJSON *to   = cJSON_Parse("{\"a\":1,\"b\":2}");

    /* Trigger patch generation; compose_patch will call malloc for full_path.
     * With a failing allocator this crashes at the sprintf line. */
    fail_next = 6; /* let a few allocs succeed, then fail on full_path */
    cJSON *patches = cJSONUtils_GeneratePatches(from, to); /* segfault here */

    cJSON_Delete(patches);
    cJSON_Delete(from);
    cJSON_Delete(to);
    return 0;
}

Expected Behaviour

compose_patch should check the return value of cJSON_malloc and return early (silently or propagating an error) if the allocation fails. This is consistent with the rest of the codebase where allocation failures are handled gracefully.

Suggested Fix

unsigned char *full_path = (unsigned char*)cJSON_malloc(path_length + suffix_length + sizeof("/"));
if (full_path == NULL)
{
    cJSON_Delete(patch);
    return;
}
sprintf((char*)full_path, "%s/", (const char*)path);
encode_string_as_pointer(full_path + path_length + 1, suffix);

Note: The early return also requires freeing patch to avoid a memory leak, similar to how other allocation failures in this file are handled.

Additional Notes

  • A similar missing-NULL-check pattern exists in create_patches() at line 1246 (new_path) and line 1175 (new_path for arrays), though those are within void functions that silently drop patches on failure—arguably acceptable but still inconsistent.
  • This bug was found by manual code review of cJSON v1.7.19.
  • This is not a duplicate of fix a possible dereference of null pointer #519 ("Fix a possible dereference of null pointer") which targeted a different code path.

Environment

  • Library version: cJSON 1.7.19
  • Compiler: GCC / Clang (any standard C compiler)
  • OS: Any (bug is platform-independent)
  • Researched By Eneshan Erdoğan Karaca

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions