From d3b24529f7f187e6a0f17d9b5d710ddd7e18e39b Mon Sep 17 00:00:00 2001 From: Nicolas Badoux Date: Fri, 23 Aug 2024 14:50:30 +0000 Subject: [PATCH 1/4] fix #881, check overlap before calling strcpy in cJSON_SetValuestring --- cJSON.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/cJSON.c b/cJSON.c index cac1164b..27402d1f 100644 --- a/cJSON.c +++ b/cJSON.c @@ -403,6 +403,8 @@ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number) CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring) { char *copy = NULL; + size_t v1_len; + size_t v2_len; /* if object's type is not cJSON_String or is cJSON_IsReference, it should not set valuestring */ if ((object == NULL) || !(object->type & cJSON_String) || (object->type & cJSON_IsReference)) { @@ -413,8 +415,17 @@ CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring) { return NULL; } - if (strlen(valuestring) <= strlen(object->valuestring)) + + v1_len = strlen(valuestring); + v2_len = strlen(object->valuestring); + + if (v1_len <= v2_len) { + /* strcpy does not handle overlapping string: [X1, X2] [Y1, Y2] => X2 < Y1 or Y2 < X1 */ + if (!( valuestring + v1_len < object->valuestring || object->valuestring + v2_len < valuestring )) + { + return NULL; + } strcpy(object->valuestring, valuestring); return object->valuestring; } From 193cd6aae9b0fc90ada8c75d4065145a0bce2006 Mon Sep 17 00:00:00 2001 From: Nicolas Badoux Date: Sun, 25 Aug 2024 22:32:02 +0200 Subject: [PATCH 2/4] CJSON_SetValuestring: add test for overlapping string --- tests/misc_tests.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index ba3e003e..dd8db181 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -456,6 +456,19 @@ static void cjson_functions_should_not_crash_with_null_pointers(void) cJSON_Delete(item); } +static void cjson_set_valuestring_should_return_null_if_strings_overlap(void) +{ + cJSON *obj, *obj_dup; + char* str; + + obj = cJSON_Parse("\"fooz\""); + obj_dup = cJSON_Duplicate(obj, 1); + + str = cJSON_SetValuestring(obj_dup, "beeez"); + cJSON_SetValuestring(obj_dup, str); + cJSON_SetValuestring(obj_dup, ++str); +} + static void *CJSON_CDECL failing_realloc(void *pointer, size_t size) { (void)size; @@ -749,6 +762,7 @@ int CJSON_CDECL main(void) RUN_TEST(cjson_replace_item_via_pointer_should_replace_items); RUN_TEST(cjson_replace_item_in_object_should_preserve_name); RUN_TEST(cjson_functions_should_not_crash_with_null_pointers); + RUN_TEST(cjson_set_valuestring_should_return_null_if_strings_overlap); RUN_TEST(ensure_should_fail_on_failed_realloc); RUN_TEST(skip_utf8_bom_should_skip_bom); RUN_TEST(skip_utf8_bom_should_not_skip_bom_if_not_at_beginning); From f0c645542d87615705f1255e104d9c0b4d95f889 Mon Sep 17 00:00:00 2001 From: Nicolas Badoux Date: Sun, 25 Aug 2024 23:06:21 +0200 Subject: [PATCH 3/4] CJSON_SetValuestring: better test for overlapping string --- tests/misc_tests.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index dd8db181..59b3a6b7 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -458,15 +458,19 @@ static void cjson_functions_should_not_crash_with_null_pointers(void) static void cjson_set_valuestring_should_return_null_if_strings_overlap(void) { - cJSON *obj, *obj_dup; + cJSON *obj; char* str; + char* str2; - obj = cJSON_Parse("\"fooz\""); - obj_dup = cJSON_Duplicate(obj, 1); + obj = cJSON_Parse("\"foo0z\""); - str = cJSON_SetValuestring(obj_dup, "beeez"); - cJSON_SetValuestring(obj_dup, str); - cJSON_SetValuestring(obj_dup, ++str); + str = cJSON_SetValuestring(obj, "abcde"); + str += 1; + /* The string passed to strcpy overlap which is not allowed.*/ + str2 = cJSON_SetValuestring(obj, str); + /* If it overlaps, the string will be messed up.*/ + TEST_ASSERT_TRUE(strcmp(str, "bcde") == 0); + TEST_ASSERT_NULL(str2); } static void *CJSON_CDECL failing_realloc(void *pointer, size_t size) From 3f09bd65f8cedece458de3bbfbd3894d24b4c3fb Mon Sep 17 00:00:00 2001 From: Nicolas Badoux Date: Mon, 26 Aug 2024 09:48:59 +0200 Subject: [PATCH 4/4] Free mem in cjson_set_valuestring_should_return_null_if_strings_overlap --- tests/misc_tests.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index 59b3a6b7..c558ac0d 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -471,6 +471,7 @@ static void cjson_set_valuestring_should_return_null_if_strings_overlap(void) /* If it overlaps, the string will be messed up.*/ TEST_ASSERT_TRUE(strcmp(str, "bcde") == 0); TEST_ASSERT_NULL(str2); + cJSON_Delete(obj); } static void *CJSON_CDECL failing_realloc(void *pointer, size_t size)