From ee29b41b040888e5b881c7c90275401027299fa6 Mon Sep 17 00:00:00 2001 From: liuyuxiu <1175395694@qq.com> Date: Thu, 12 Jun 2025 15:30:06 +0800 Subject: [PATCH 1/3] =?UTF-8?q?CVE-2023-26819=20=E6=BC=8F=E6=B4=9E?= =?UTF-8?q?=E4=BF=AE=E5=A4=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: liuyuxiu <1175395694@qq.com> --- cJSON.c | 37 ++++++++++++++++++++---- diff.diff | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 diff.diff diff --git a/cJSON.c b/cJSON.c index 63fb043..e8ae047 100644 --- a/cJSON.c +++ b/cJSON.c @@ -424,9 +424,11 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu { double number = 0; unsigned char *after_end = NULL; - unsigned char number_c_string[64]; + unsigned char *number_c_string; unsigned char decimal_point = get_decimal_point(); size_t i = 0; + size_t number_string_length = 0; + cJSON_bool has_decimal_point = false; if ((input_buffer == NULL) || (input_buffer->content == NULL)) { @@ -436,7 +438,7 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu /* copy the number into a temporary buffer and replace '.' with the decimal point * of the current locale (for strtod) * This also takes care of '\0' not necessarily being available for marking the end of the input */ - for (i = 0; (i < (sizeof(number_c_string) - 1)) && can_access_at_index(input_buffer, i); i++) + for (i = 0; can_access_at_index(input_buffer, i); i++) { switch (buffer_at_offset(input_buffer)[i]) { @@ -454,11 +456,12 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu case '-': case 'e': case 'E': - number_c_string[i] = buffer_at_offset(input_buffer)[i]; + number_string_length++; break; case '.': - number_c_string[i] = decimal_point; + number_string_length++; + has_decimal_point = true; break; default: @@ -466,11 +469,33 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu } } loop_end: - number_c_string[i] = '\0'; + /* malloc for temporary buffer, add 1 for '\0' */ + number_c_string = (unsigned char *) input_buffer->hooks.allocate(number_string_length + 1); + if (number_c_string == NULL) + { + return false; /* allocation failure */ + } + + memcpy(number_c_string, buffer_at_offset(input_buffer), number_string_length); + number_c_string[number_string_length] = '\0'; + + if (has_decimal_point) + { + for (i = 0; i < number_string_length; i++) + { + if (number_c_string[i] == '.') + { + /* replace '.' with the decimal point of the current locale (for strtod) */ + number_c_string[i] = decimal_point; + } + } + } number = strtod((const char*)number_c_string, (char**)&after_end); if (number_c_string == after_end) { + /* free the temporary buffer */ + input_buffer->hooks.deallocate(number_c_string); return false; /* parse_error */ } @@ -493,6 +518,8 @@ loop_end: item->type = cJSON_Number; input_buffer->offset += (size_t)(after_end - number_c_string); + /* free the temporary buffer */ + input_buffer->hooks.deallocate(number_c_string); return true; } #endif /* __CJSON_USE_INT64 */ diff --git a/diff.diff b/diff.diff new file mode 100644 index 0000000..a532a34 --- /dev/null +++ b/diff.diff @@ -0,0 +1,85 @@ +diff --git a/cJSON.c b/cJSON.c +index cbf458cd9e48802dd1cfad2daafe7ca139e9bc22..d8ce5df5f8a2f8bd49bfbc0e6f566b212ef7f84c 100644 +--- a/cJSON.c ++++ b/cJSON.c +@@ -312,9 +312,11 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu + { + double number = 0; + unsigned char *after_end = NULL; +- unsigned char number_c_string[64]; ++ unsigned char *number_c_string; + unsigned char decimal_point = get_decimal_point(); + size_t i = 0; ++ size_t number_string_length = 0; ++ cJSON_bool has_decimal_point = false; + + if ((input_buffer == NULL) || (input_buffer->content == NULL)) + { +@@ -324,7 +326,7 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu + /* copy the number into a temporary buffer and replace '.' with the decimal point + * of the current locale (for strtod) + * This also takes care of '\0' not necessarily being available for marking the end of the input */ +- for (i = 0; (i < (sizeof(number_c_string) - 1)) && can_access_at_index(input_buffer, i); i++) ++ for (i = 0; can_access_at_index(input_buffer, i); i++) + { + switch (buffer_at_offset(input_buffer)[i]) + { +@@ -342,11 +344,12 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu + case '-': + case 'e': + case 'E': +- number_c_string[i] = buffer_at_offset(input_buffer)[i]; ++ number_string_length++; + break; + + case '.': +- number_c_string[i] = decimal_point; ++ number_string_length++; ++ has_decimal_point = true; + break; + + default: +@@ -354,11 +357,34 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu + } + } + loop_end: ++ /* malloc for temporary buffer, add 1 for '\0' */ ++ number_c_string = (unsigned char *) input_buffer->hooks.allocate(number_string_length + 1); ++ if (number_c_string == NULL) ++ { ++ return false; /* allocation failure */ ++ } ++ ++ memcpy(number_c_string, buffer_at_offset(input_buffer), number_string_length); ++ number_c_string[number_string_length] = '\0'; ++ ++ if (has_decimal_point) ++ { ++ for (i = 0; i < number_string_length; i++) ++ { ++ if (number_c_string[i] == '.') ++ { ++ /* replace '.' with the decimal point of the current locale (for strtod) */ ++ number_c_string[i] = decimal_point; ++ } ++ } ++ } + number_c_string[i] = '\0'; + + number = strtod((const char*)number_c_string, (char**)&after_end); + if (number_c_string == after_end) + { ++ /* free the temporary buffer */ ++ input_buffer->hooks.deallocate(number_c_string); + return false; /* parse_error */ + } + +@@ -381,6 +407,8 @@ loop_end: + item->type = cJSON_Number; + + input_buffer->offset += (size_t)(after_end - number_c_string); ++ /* free the temporary buffer */ ++ input_buffer->hooks.deallocate(number_c_string); + return true; + } + -- Gitee From 81a983724159f903d878c0bb1c57dc9911180266 Mon Sep 17 00:00:00 2001 From: liuyuxiu <1175395694@qq.com> Date: Thu, 12 Jun 2025 17:29:47 +0800 Subject: [PATCH 2/3] bugfix Signed-off-by: liuyuxiu <1175395694@qq.com> --- tests/misc_tests.c | 17 +++++++++++++++++ tests/parse_number.c | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index 2a79dc0..44a7318 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -741,6 +741,22 @@ static void cjson_set_bool_value_must_not_break_objects(void) cJSON_Delete(sobj); } +static void cjson_parse_big_numbers_should_not_report_error(void) +{ + cJSON *valid_big_number_json_object1 = cJSON_Parse("{\"a\": true, \"b\": [ null,9999999999999999999999999999999999999999999999912345678901234567]}"); + cJSON *valid_big_number_json_object2 = cJSON_Parse("{\"a\": true, \"b\": [ null,999999999999999999999999999999999999999999999991234567890.1234567E3]}"); + const char *invalid_big_number_json1 = "{\"a\": true, \"b\": [ null,99999999999999999999999999999999999999999999999.1234567890.1234567]}"; + const char *invalid_big_number_json2 = "{\"a\": true, \"b\": [ null,99999999999999999999999999999999999999999999999E1234567890e1234567]}"; + + TEST_ASSERT_NOT_NULL(valid_big_number_json_object1); + TEST_ASSERT_NOT_NULL(valid_big_number_json_object2); + TEST_ASSERT_NULL_MESSAGE(cJSON_Parse(invalid_big_number_json1), "Invalid big number JSONs should not be parsed."); + TEST_ASSERT_NULL_MESSAGE(cJSON_Parse(invalid_big_number_json2), "Invalid big number JSONs should not be parsed."); + + cJSON_Delete(valid_big_number_json_object1); + cJSON_Delete(valid_big_number_json_object2); +} + int CJSON_CDECL main(void) { UNITY_BEGIN(); @@ -771,6 +787,7 @@ int CJSON_CDECL main(void) RUN_TEST(cjson_delete_item_from_array_should_not_broken_list_structure); RUN_TEST(cjson_set_valuestring_to_object_should_not_leak_memory); RUN_TEST(cjson_set_bool_value_must_not_break_objects); + RUN_TEST(cjson_parse_big_numbers_should_not_report_error); return UNITY_END(); } diff --git a/tests/parse_number.c b/tests/parse_number.c index b92162b..f04c233 100644 --- a/tests/parse_number.c +++ b/tests/parse_number.c @@ -48,6 +48,7 @@ static void assert_parse_number(const char *string, int integer, double real) parse_buffer buffer = { 0, 0, 0, 0, { 0, 0, 0 } }; buffer.content = (const unsigned char*)string; buffer.length = strlen(string) + sizeof(""); + buffer.hooks = global_hooks; TEST_ASSERT_TRUE(parse_number(item, &buffer)); assert_is_number(item); @@ -55,6 +56,17 @@ static void assert_parse_number(const char *string, int integer, double real) TEST_ASSERT_EQUAL_DOUBLE(real, item->valuedouble); } +static void assert_parse_big_number(const char *string) +{ + parse_buffer buffer = { 0, 0, 0, 0, { 0, 0, 0 } }; + buffer.content = (const unsigned char*)string; + buffer.length = strlen(string) + sizeof(""); + buffer.hooks = global_hooks; + + TEST_ASSERT_TRUE(parse_number(item, &buffer)); + assert_is_number(item); +} + #ifdef __CJSON_USE_INT64 static void assert_is_int64(cJSON *int64_number_item) { @@ -145,6 +157,13 @@ static void parse_number_should_parse_int64_numbers(void) } #endif /* __CJSON_USE_INT64 */ +static void parse_number_should_parse_big_numbers(void) +{ + assert_parse_big_number("9999999999999999999999999999999999999999999999912345678901234567"); + assert_parse_big_number("9999999999999999999999999999999999999999999999912345678901234567E10"); + assert_parse_big_number("999999999999999999999999999999999999999999999991234567890.1234567"); +} + int CJSON_CDECL main(void) { /* initialize cJSON item */ @@ -158,5 +177,6 @@ int CJSON_CDECL main(void) #ifdef __CJSON_USE_INT64 RUN_TEST(parse_number_should_parse_int64_numbers); #endif + RUN_TEST(parse_number_should_parse_big_numbers); return UNITY_END(); } -- Gitee From 6b4e32cc402985028a8062e98908614a03c4485a Mon Sep 17 00:00:00 2001 From: liuyuxiu Date: Thu, 12 Jun 2025 09:30:59 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=E5=88=A0=E9=99=A4=E6=96=87=E4=BB=B6=20diff?= =?UTF-8?q?.diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- diff.diff | 85 ------------------------------------------------------- 1 file changed, 85 deletions(-) delete mode 100644 diff.diff diff --git a/diff.diff b/diff.diff deleted file mode 100644 index a532a34..0000000 --- a/diff.diff +++ /dev/null @@ -1,85 +0,0 @@ -diff --git a/cJSON.c b/cJSON.c -index cbf458cd9e48802dd1cfad2daafe7ca139e9bc22..d8ce5df5f8a2f8bd49bfbc0e6f566b212ef7f84c 100644 ---- a/cJSON.c -+++ b/cJSON.c -@@ -312,9 +312,11 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu - { - double number = 0; - unsigned char *after_end = NULL; -- unsigned char number_c_string[64]; -+ unsigned char *number_c_string; - unsigned char decimal_point = get_decimal_point(); - size_t i = 0; -+ size_t number_string_length = 0; -+ cJSON_bool has_decimal_point = false; - - if ((input_buffer == NULL) || (input_buffer->content == NULL)) - { -@@ -324,7 +326,7 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu - /* copy the number into a temporary buffer and replace '.' with the decimal point - * of the current locale (for strtod) - * This also takes care of '\0' not necessarily being available for marking the end of the input */ -- for (i = 0; (i < (sizeof(number_c_string) - 1)) && can_access_at_index(input_buffer, i); i++) -+ for (i = 0; can_access_at_index(input_buffer, i); i++) - { - switch (buffer_at_offset(input_buffer)[i]) - { -@@ -342,11 +344,12 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu - case '-': - case 'e': - case 'E': -- number_c_string[i] = buffer_at_offset(input_buffer)[i]; -+ number_string_length++; - break; - - case '.': -- number_c_string[i] = decimal_point; -+ number_string_length++; -+ has_decimal_point = true; - break; - - default: -@@ -354,11 +357,34 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu - } - } - loop_end: -+ /* malloc for temporary buffer, add 1 for '\0' */ -+ number_c_string = (unsigned char *) input_buffer->hooks.allocate(number_string_length + 1); -+ if (number_c_string == NULL) -+ { -+ return false; /* allocation failure */ -+ } -+ -+ memcpy(number_c_string, buffer_at_offset(input_buffer), number_string_length); -+ number_c_string[number_string_length] = '\0'; -+ -+ if (has_decimal_point) -+ { -+ for (i = 0; i < number_string_length; i++) -+ { -+ if (number_c_string[i] == '.') -+ { -+ /* replace '.' with the decimal point of the current locale (for strtod) */ -+ number_c_string[i] = decimal_point; -+ } -+ } -+ } - number_c_string[i] = '\0'; - - number = strtod((const char*)number_c_string, (char**)&after_end); - if (number_c_string == after_end) - { -+ /* free the temporary buffer */ -+ input_buffer->hooks.deallocate(number_c_string); - return false; /* parse_error */ - } - -@@ -381,6 +407,8 @@ loop_end: - item->type = cJSON_Number; - - input_buffer->offset += (size_t)(after_end - number_c_string); -+ /* free the temporary buffer */ -+ input_buffer->hooks.deallocate(number_c_string); - return true; - } - -- Gitee