diff --git a/cJSON.c b/cJSON.c index 88c2d95b..45f90325 100644 --- a/cJSON.c +++ b/cJSON.c @@ -3069,13 +3069,18 @@ CJSON_PUBLIC(cJSON_bool) cJSON_IsRaw(const cJSON * const item) return (item->type & 0xFF) == cJSON_Raw; } -CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * const b, const cJSON_bool case_sensitive) +static cJSON_bool cJSON_Compare_rec(const cJSON * const a, const cJSON * const b, const cJSON_bool case_sensitive, size_t depth) { if ((a == NULL) || (b == NULL) || ((a->type & 0xFF) != (b->type & 0xFF))) { return false; } + if (depth >= CJSON_NESTING_LIMIT) + { + return false; + } + /* check if type is valid */ switch (a->type & 0xFF) { @@ -3134,7 +3139,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * cons for (; (a_element != NULL) && (b_element != NULL);) { - if (!cJSON_Compare(a_element, b_element, case_sensitive)) + if (!cJSON_Compare_rec(a_element, b_element, case_sensitive, depth + 1)) { return false; } @@ -3164,7 +3169,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * cons return false; } - if (!cJSON_Compare(a_element, b_element, case_sensitive)) + if (!cJSON_Compare_rec(a_element, b_element, case_sensitive, depth + 1)) { return false; } @@ -3180,7 +3185,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * cons return false; } - if (!cJSON_Compare(b_element, a_element, case_sensitive)) + if (!cJSON_Compare_rec(b_element, a_element, case_sensitive, depth + 1)) { return false; } @@ -3194,6 +3199,11 @@ CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * cons } } +CJSON_PUBLIC(cJSON_bool) cJSON_Compare(const cJSON * const a, const cJSON * const b, const cJSON_bool case_sensitive) +{ + return cJSON_Compare_rec(a, b, case_sensitive, 0); +} + CJSON_PUBLIC(void *) cJSON_malloc(size_t size) { return global_hooks.allocate(size); diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 8b38eb25..8b8abff4 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -601,13 +601,18 @@ 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, const cJSON_bool case_sensitive, size_t depth) { if ((a == NULL) || (b == NULL) || ((a->type & 0xFF) != (b->type & 0xFF))) { /* mismatched type. */ return false; } + + if (depth >= CJSON_NESTING_LIMIT) + { + return false; + } switch (a->type & 0xFF) { case cJSON_Number: @@ -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, case_sensitive, depth + 1); 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, case_sensitive, depth + 1); 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), case_sensitive, 0); goto cleanup; } @@ -1388,16 +1393,22 @@ CJSON_PUBLIC(cJSON *) cJSONUtils_MergePatchCaseSensitive(cJSON *target, const cJ return merge_patch(target, patch, 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, const cJSON_bool case_sensitive, size_t depth) { cJSON *from_child = NULL; cJSON *to_child = NULL; cJSON *patch = NULL; + cJSON *child_patch = NULL; if (to == NULL) { /* patch to delete everything */ return cJSON_CreateNull(); } + if (depth >= CJSON_NESTING_LIMIT) + { + /* Do not recurse beyond the same limit used by cJSON comparisons. */ + return NULL; + } if (!cJSON_IsObject(to) || !cJSON_IsObject(from)) { return cJSON_Duplicate(to, 1); @@ -1449,10 +1460,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, case_sensitive, 0)) { /* not identical --> generate a patch */ - cJSON_AddItemToObject(patch, to_child->string, cJSONUtils_GenerateMergePatch(from_child, to_child)); + child_patch = generate_merge_patch(from_child, to_child, case_sensitive, depth + 1); + if ((child_patch == NULL) || !cJSON_AddItemToObject(patch, to_child->string, child_patch)) + { + cJSON_Delete(child_patch); + cJSON_Delete(patch); + return NULL; + } } /* next key in the object */ @@ -1472,10 +1489,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, false, 0); } CJSON_PUBLIC(cJSON *) cJSONUtils_GenerateMergePatchCaseSensitive(cJSON * const from, cJSON * const to) { - return generate_merge_patch(from, to, true); + return generate_merge_patch(from, to, true, 0); } diff --git a/tests/compare_tests.c b/tests/compare_tests.c index 797c7740..a9346632 100644 --- a/tests/compare_tests.c +++ b/tests/compare_tests.c @@ -189,6 +189,59 @@ static void cjson_compare_should_compare_objects(void) false)) } +static cJSON *create_nested_arrays(const size_t depth) +{ + cJSON *root = cJSON_CreateArray(); + cJSON *current = root; + size_t i; + + if (root == NULL) + { + return NULL; + } + + for (i = 0; i < depth; i++) + { + cJSON *child = cJSON_CreateArray(); + if (child == NULL) + { + cJSON_Delete(root); + return NULL; + } + cJSON_AddItemToArray(current, child); + current = child; + } + + if (cJSON_AddItemToArray(current, cJSON_CreateNumber(1)) == false) + { + cJSON_Delete(root); + return NULL; + } + + return root; +} + +static void cjson_compare_should_bound_recursive_comparisons(void) +{ + cJSON *within_limit = create_nested_arrays(CJSON_NESTING_LIMIT - 2); + cJSON *within_limit_copy = create_nested_arrays(CJSON_NESTING_LIMIT - 2); + cJSON *at_limit = create_nested_arrays(CJSON_NESTING_LIMIT - 1); + cJSON *at_limit_copy = create_nested_arrays(CJSON_NESTING_LIMIT - 1); + + TEST_ASSERT_NOT_NULL(within_limit); + TEST_ASSERT_NOT_NULL(within_limit_copy); + TEST_ASSERT_NOT_NULL(at_limit); + TEST_ASSERT_NOT_NULL(at_limit_copy); + + TEST_ASSERT_TRUE(cJSON_Compare(within_limit, within_limit_copy, true)); + TEST_ASSERT_FALSE(cJSON_Compare(at_limit, at_limit_copy, true)); + + cJSON_Delete(within_limit); + cJSON_Delete(within_limit_copy); + cJSON_Delete(at_limit); + cJSON_Delete(at_limit_copy); +} + int CJSON_CDECL main(void) { UNITY_BEGIN(); @@ -203,6 +256,7 @@ int CJSON_CDECL main(void) RUN_TEST(cjson_compare_should_compare_raw); RUN_TEST(cjson_compare_should_compare_arrays); RUN_TEST(cjson_compare_should_compare_objects); + RUN_TEST(cjson_compare_should_bound_recursive_comparisons); return UNITY_END(); } diff --git a/tests/misc_utils_tests.c b/tests/misc_utils_tests.c index 7d300bc8..3d92d9ab 100644 --- a/tests/misc_utils_tests.c +++ b/tests/misc_utils_tests.c @@ -70,11 +70,60 @@ static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static cJSON *create_nested_objects(const size_t depth) +{ + cJSON *root = cJSON_CreateObject(); + cJSON *current = root; + size_t i; + + if (root == NULL) + { + return NULL; + } + + for (i = 0; i < depth; i++) + { + cJSON *child = cJSON_CreateObject(); + if (child == NULL || cJSON_AddItemToObject(current, "child", child) == false) + { + cJSON_Delete(child); + cJSON_Delete(root); + return NULL; + } + current = child; + } + + if (cJSON_AddNumberToObject(current, "value", 1) == NULL) + { + cJSON_Delete(root); + return NULL; + } + + return root; +} + +static void cjson_utils_should_propagate_bounded_merge_patch_failure(void) +{ + cJSON *from = create_nested_objects(CJSON_NESTING_LIMIT); + cJSON *to = create_nested_objects(CJSON_NESTING_LIMIT); + cJSON *patch = NULL; + + TEST_ASSERT_NOT_NULL(from); + TEST_ASSERT_NOT_NULL(to); + + patch = cJSONUtils_GenerateMergePatch(from, to); + TEST_ASSERT_NULL(patch); + + 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_should_propagate_bounded_merge_patch_failure); return UNITY_END(); }