From f5ff7168a6ff111f6a3c681ef2099ffc239f873d Mon Sep 17 00:00:00 2001 From: JIgar-maheshwari-dev Date: Wed, 22 Jul 2026 00:11:01 +0530 Subject: [PATCH] fix(cjson_utils): add NULL check for cJSON_malloc in FindPointerFromObjectTo (#1049) - Add missing NULL check for full_pointer allocation to prevent SIGSEGV on OOM. - Free target_pointer and return NULL safely if full_pointer fails to allocate. - Add oom_test unit test to verify graceful handling of memory allocation failure. Signed-off-by: JIgar-maheshwari-dev --- cJSON_Utils.c | 29 +++++++++++++++- tests/CMakeLists.txt | 3 +- tests/oom_test.c | 82 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 tests/oom_test.c diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 0d199f883..4ed2f7134 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -203,6 +203,12 @@ CJSON_PUBLIC(char *) cJSONUtils_FindPointerFromObjectTo(const cJSON * const obje { /* reserve enough memory for a 64 bit integer + '/' and '\0' */ unsigned char *full_pointer = (unsigned char*)cJSON_malloc(strlen((char*)target_pointer) + 20 + sizeof("/")); + if(full_pointer == NULL) + { + /* Return early on allocation failure to prevent crash */ + cJSON_free(target_pointer); + return NULL; + } /* check if conversion to unsigned long is valid * This should be eliminated at compile time by dead code elimination * if size_t is an alias of unsigned long, or if it is bigger */ @@ -220,6 +226,12 @@ CJSON_PUBLIC(char *) cJSONUtils_FindPointerFromObjectTo(const cJSON * const obje if (cJSON_IsObject(object)) { unsigned char *full_pointer = (unsigned char*)cJSON_malloc(strlen((char*)target_pointer) + pointer_encoded_length((unsigned char*)current_child->string) + 2); + if(full_pointer == NULL) + { + /* Return early on allocation failure to prevent crash */ + cJSON_free(target_pointer); + return NULL; + } full_pointer[0] = '/'; encode_string_as_pointer(full_pointer + 1, (unsigned char*)current_child->string); strcat((char*)full_pointer, (char*)target_pointer); @@ -1091,6 +1103,12 @@ static void compose_patch(cJSON * const patches, const unsigned char * const ope size_t suffix_length = pointer_encoded_length(suffix); size_t path_length = strlen((const char*)path); unsigned char *full_path = (unsigned char*)cJSON_malloc(path_length + suffix_length + sizeof("/")); + if(full_path == NULL) + { + /* Return early on allocation failure to prevent crash */ + cJSON_Delete(patch); + return; + } sprintf((char*)full_path, "%s/", (const char*)path); encode_string_as_pointer(full_path + path_length + 1, suffix); @@ -1146,6 +1164,11 @@ static void create_patches(cJSON * const patches, const unsigned char * const pa cJSON *from_child = from->child; cJSON *to_child = to->child; unsigned char *new_path = (unsigned char*)cJSON_malloc(strlen((const char*)path) + 20 + sizeof("/")); /* Allow space for 64bit int. log10(2^64) = 20 */ + if(new_path == NULL) + { + /* Return early on allocation failure to prevent crash */ + return; + } /* generate patches for all array elements that exist in both "from" and "to" */ for (index = 0; (from_child != NULL) && (to_child != NULL); (void)(from_child = from_child->next), (void)(to_child = to_child->next), index++) @@ -1217,7 +1240,11 @@ static void create_patches(cJSON * const patches, const unsigned char * const pa size_t path_length = strlen((const char*)path); size_t from_child_name_length = pointer_encoded_length((unsigned char*)from_child->string); unsigned char *new_path = (unsigned char*)cJSON_malloc(path_length + from_child_name_length + sizeof("/")); - + if(new_path == NULL) + { + /* Return early on allocation failure to prevent crash */ + return; + } sprintf((char*)new_path, "%s/", path); encode_string_as_pointer(new_path + path_length + 1, (unsigned char*)from_child->string); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 79672927e..deec28888 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -93,7 +93,8 @@ if(ENABLE_CJSON_TEST) set (cjson_utils_tests json_patch_tests old_utils_tests - misc_utils_tests) + misc_utils_tests + oom_test) foreach (cjson_utils_test ${cjson_utils_tests}) add_executable("${cjson_utils_test}" "${cjson_utils_test}.c") diff --git a/tests/oom_test.c b/tests/oom_test.c new file mode 100644 index 000000000..2f62d7c6a --- /dev/null +++ b/tests/oom_test.c @@ -0,0 +1,82 @@ +#include +#include +#include + +#include "unity/examples/unity_config.h" +#include "unity/src/unity.h" +#include "../cJSON.h" +#include "../cJSON_Utils.h" +#include "common.h" + +static int g_alloc_count = 0; +static int g_fail_at = -1; + +/* Custom malloc hook to simulate OOM on a specific allocation */ +static void* failing_malloc(size_t size) +{ + int idx = g_alloc_count++; + if (idx == g_fail_at) + { + return NULL; /* Simulate allocation failure */ + } + return malloc(size); +} + +void setUp(void) +{ + /* Declare variables at top of block */ + cJSON_Hooks hooks; + + hooks.malloc_fn = failing_malloc; + hooks.free_fn = free; + + g_alloc_count = 0; + g_fail_at = -1; + + /* Register custom memory hook */ + cJSON_InitHooks(&hooks); +} + +void tearDown(void) +{ + /* Reset hooks back to default */ + cJSON_InitHooks(NULL); +} + +static void find_pointer_should_handle_null_on_oom(void) +{ + /* 1. ALL variable declarations MUST be at the very top */ + cJSON *root = NULL; + cJSON *inner = NULL; + cJSON *target = NULL; + char *pointer = NULL; + + /* 2. Executable code starts here */ + root = cJSON_CreateArray(); + inner = cJSON_CreateArray(); + cJSON_AddItemToArray(root, inner); + + target = cJSON_CreateNumber(42); + cJSON_AddItemToArray(inner, target); + + /* Reset alloc counter right before calling FindPointer */ + g_alloc_count = 0; + g_fail_at = 1; + + pointer = cJSONUtils_FindPointerFromObjectTo(root, target); + + /* Assertion */ + TEST_ASSERT_NULL(pointer); + + /* Cleanup memory */ + cJSON_Delete(root); +} + +int CJSON_CDECL main(void) +{ + UNITY_BEGIN(); + + RUN_TEST(find_pointer_should_handle_null_on_oom); + + return UNITY_END(); +}