diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 8b38eb253..2d044edcd 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -601,8 +601,13 @@ static void sort_object(cJSON * const object, const cJSON_bool case_sensitive) object->child = sort_list(object->child, case_sensitive); } -static cJSON_bool compare_json(cJSON *a, cJSON *b, const cJSON_bool case_sensitive) +static cJSON_bool compare_json(cJSON *a, cJSON *b, size_t depth, const cJSON_bool case_sensitive) { + /* Prevent stack overflow from deeply nested JSON comparison (#995) */ + if (depth >= CJSON_NESTING_LIMIT) + { + return false; + } if ((a == NULL) || (b == NULL) || ((a->type & 0xFF) != (b->type & 0xFF))) { /* mismatched type. */ @@ -635,7 +640,7 @@ static cJSON_bool compare_json(cJSON *a, cJSON *b, const cJSON_bool case_sensiti case cJSON_Array: for ((void)(a = a->child), b = b->child; (a != NULL) && (b != NULL); (void)(a = a->next), b = b->next) { - cJSON_bool identical = compare_json(a, b, case_sensitive); + cJSON_bool identical = compare_json(a, b, depth + 1, case_sensitive); if (!identical) { return false; @@ -664,7 +669,7 @@ static cJSON_bool compare_json(cJSON *a, cJSON *b, const cJSON_bool case_sensiti /* missing member */ return false; } - identical = compare_json(a, b, case_sensitive); + identical = compare_json(a, b, depth + 1, case_sensitive); if (!identical) { return false; @@ -831,7 +836,7 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_ else if (opcode == TEST) { /* compare value: {...} with the given path */ - status = !compare_json(get_item_from_pointer(object, path->valuestring, case_sensitive), get_object_item(patch, "value", case_sensitive), case_sensitive); + status = !compare_json(get_item_from_pointer(object, path->valuestring, case_sensitive), get_object_item(patch, "value", case_sensitive), 0, case_sensitive); goto cleanup; } @@ -1318,10 +1323,17 @@ CJSON_PUBLIC(void) cJSONUtils_SortObjectCaseSensitive(cJSON * const object) sort_object(object, true); } -static cJSON *merge_patch(cJSON *target, const cJSON * const patch, const cJSON_bool case_sensitive) +static cJSON *merge_patch(cJSON *target, const cJSON * const patch, size_t depth, const cJSON_bool case_sensitive) { cJSON *patch_child = NULL; + /* Prevent stack overflow from deeply nested JSON merge patch (#995) */ + if (depth >= CJSON_NESTING_LIMIT) + { + cJSON_Delete(target); + return NULL; + } + if (!cJSON_IsObject(patch)) { /* scalar value, array or NULL, just duplicate */ @@ -1364,7 +1376,7 @@ static cJSON *merge_patch(cJSON *target, const cJSON * const patch, const cJSON_ replace_me = cJSON_DetachItemFromObject(target, patch_child->string); } - replacement = merge_patch(replace_me, patch_child, case_sensitive); + replacement = merge_patch(replace_me, patch_child, depth + 1, case_sensitive); if (replacement == NULL) { cJSON_Delete(target); @@ -1380,19 +1392,25 @@ static cJSON *merge_patch(cJSON *target, const cJSON * const patch, const cJSON_ CJSON_PUBLIC(cJSON *) cJSONUtils_MergePatch(cJSON *target, const cJSON * const patch) { - return merge_patch(target, patch, false); + return merge_patch(target, patch, 0, false); } CJSON_PUBLIC(cJSON *) cJSONUtils_MergePatchCaseSensitive(cJSON *target, const cJSON * const patch) { - return merge_patch(target, patch, true); + return merge_patch(target, patch, 0, true); } -static cJSON *generate_merge_patch(cJSON * const from, cJSON * const to, const cJSON_bool case_sensitive) +static cJSON *generate_merge_patch(cJSON * const from, cJSON * const to, size_t depth, const cJSON_bool case_sensitive) { cJSON *from_child = NULL; cJSON *to_child = NULL; cJSON *patch = NULL; + cJSON *sub_patch = NULL; + /* Prevent stack overflow from deeply nested JSON generate merge patch (#995) */ + if (depth >= CJSON_NESTING_LIMIT) + { + return NULL; + } if (to == NULL) { /* patch to delete everything */ @@ -1449,10 +1467,16 @@ static cJSON *generate_merge_patch(cJSON * const from, cJSON * const to, const c else { /* object key exists in both objects */ - if (!compare_json(from_child, to_child, case_sensitive)) + if (!compare_json(from_child, to_child, 0, case_sensitive)) { + sub_patch = generate_merge_patch(from_child, to_child, depth + 1, case_sensitive); + if (sub_patch == NULL) + { + cJSON_Delete(patch); + return NULL; + } /* not identical --> generate a patch */ - cJSON_AddItemToObject(patch, to_child->string, cJSONUtils_GenerateMergePatch(from_child, to_child)); + cJSON_AddItemToObject(patch, to_child->string, sub_patch); } /* next key in the object */ @@ -1472,10 +1496,10 @@ static cJSON *generate_merge_patch(cJSON * const from, cJSON * const to, const c CJSON_PUBLIC(cJSON *) cJSONUtils_GenerateMergePatch(cJSON * const from, cJSON * const to) { - return generate_merge_patch(from, to, false); + return generate_merge_patch(from, to, 0, false); } CJSON_PUBLIC(cJSON *) cJSONUtils_GenerateMergePatchCaseSensitive(cJSON * const from, cJSON * const to) { - return generate_merge_patch(from, to, true); + return generate_merge_patch(from, to, 0, true); } diff --git a/tests/misc_utils_tests.c b/tests/misc_utils_tests.c index 7d300bc8e..c721b0052 100644 --- a/tests/misc_utils_tests.c +++ b/tests/misc_utils_tests.c @@ -29,6 +29,25 @@ #include "common.h" #include "../cJSON_Utils.h" +#define TEST_NESTING_LIMIT 2000 + +static cJSON *create_deeply_nested_object(size_t depth) +{ + cJSON *root = cJSON_CreateObject(); + cJSON *curr = root; + cJSON *next = NULL; + size_t i = 0; + + for (i = 0; (i < depth) && (curr != NULL); ++i) + { + next = cJSON_CreateObject(); + cJSON_AddItemToObject(curr, "a", next); + curr = next; + } + + return root; +} + static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) { cJSON *item = cJSON_CreateString("item"); @@ -70,11 +89,42 @@ static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static void cjson_utils_merge_patch_should_not_overflow_stack_on_deep_nesting(void) +{ + cJSON *target = cJSON_CreateObject(); + cJSON *patch = create_deeply_nested_object(TEST_NESTING_LIMIT); + + TEST_ASSERT_NOT_NULL(target); + TEST_ASSERT_NOT_NULL(patch); + + TEST_ASSERT_NULL(cJSONUtils_MergePatch(target, patch)); + + cJSON_Delete(patch); +} + +static void cjson_utils_generate_merge_patch_should_not_overflow_stack_on_deep_nesting(void) +{ + cJSON *from = create_deeply_nested_object(TEST_NESTING_LIMIT); + cJSON *to = create_deeply_nested_object(TEST_NESTING_LIMIT); + + TEST_ASSERT_NOT_NULL(from); + TEST_ASSERT_NOT_NULL(to); + + cJSON_AddNumberToObject(to, "diff", 123); + + TEST_ASSERT_NULL(cJSONUtils_GenerateMergePatch(from, to)); + + cJSON_Delete(from); + cJSON_Delete(to); +} + int main(void) { UNITY_BEGIN(); RUN_TEST(cjson_utils_functions_shouldnt_crash_with_null_pointers); + RUN_TEST(cjson_utils_merge_patch_should_not_overflow_stack_on_deep_nesting); + RUN_TEST(cjson_utils_generate_merge_patch_should_not_overflow_stack_on_deep_nesting); return UNITY_END(); }