Skip to content

Commit a328d65

Browse files
PeterAlfredLeeAlanscut
authored andcommitted
allocate memory for the temporary buffer
Allocate memory for the temporary buffer when paring numbers. This fixes CVE-2023-26819
1 parent 12c4bf1 commit a328d65

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

cJSON.c

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,11 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu
308308
{
309309
double number = 0;
310310
unsigned char *after_end = NULL;
311-
unsigned char number_c_string[64];
311+
unsigned char *number_c_string;
312312
unsigned char decimal_point = get_decimal_point();
313313
size_t i = 0;
314+
size_t number_string_length = 0;
315+
cJSON_bool has_decimal_point = false;
314316

315317
if ((input_buffer == NULL) || (input_buffer->content == NULL))
316318
{
@@ -320,7 +322,7 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu
320322
/* copy the number into a temporary buffer and replace '.' with the decimal point
321323
* of the current locale (for strtod)
322324
* This also takes care of '\0' not necessarily being available for marking the end of the input */
323-
for (i = 0; (i < (sizeof(number_c_string) - 1)) && can_access_at_index(input_buffer, i); i++)
325+
for (i = 0; can_access_at_index(input_buffer, i); i++)
324326
{
325327
switch (buffer_at_offset(input_buffer)[i])
326328
{
@@ -338,23 +340,46 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu
338340
case '-':
339341
case 'e':
340342
case 'E':
341-
number_c_string[i] = buffer_at_offset(input_buffer)[i];
343+
number_string_length++;
342344
break;
343345

344346
case '.':
345-
number_c_string[i] = decimal_point;
347+
number_string_length++;
348+
has_decimal_point = true;
346349
break;
347350

348351
default:
349352
goto loop_end;
350353
}
351354
}
352355
loop_end:
353-
number_c_string[i] = '\0';
356+
/* malloc for temporary buffer, add 1 for '\0' */
357+
number_c_string = (unsigned char *) input_buffer->hooks.allocate(number_string_length + 1);
358+
if (number_c_string == NULL)
359+
{
360+
return false; /* allocation failure */
361+
}
362+
363+
memcpy(number_c_string, buffer_at_offset(input_buffer), number_string_length);
364+
number_c_string[number_string_length] = '\0';
365+
366+
if (has_decimal_point)
367+
{
368+
for (i = 0; i < number_string_length; i++)
369+
{
370+
if (number_c_string[i] == '.')
371+
{
372+
/* replace '.' with the decimal point of the current locale (for strtod) */
373+
number_c_string[i] = decimal_point;
374+
}
375+
}
376+
}
354377

355378
number = strtod((const char*)number_c_string, (char**)&after_end);
356379
if (number_c_string == after_end)
357380
{
381+
/* free the temporary buffer */
382+
input_buffer->hooks.deallocate(number_c_string);
358383
return false; /* parse_error */
359384
}
360385

@@ -377,6 +402,8 @@ static cJSON_bool parse_number(cJSON * const item, parse_buffer * const input_bu
377402
item->type = cJSON_Number;
378403

379404
input_buffer->offset += (size_t)(after_end - number_c_string);
405+
/* free the temporary buffer */
406+
input_buffer->hooks.deallocate(number_c_string);
380407
return true;
381408
}
382409

tests/misc_tests.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,22 @@ static void cjson_set_bool_value_must_not_break_objects(void)
782782
cJSON_Delete(sobj);
783783
}
784784

785+
static void cjson_parse_big_numbers_should_not_report_error(void)
786+
{
787+
cJSON *valid_big_number_json_object1 = cJSON_Parse("{\"a\": true, \"b\": [ null,9999999999999999999999999999999999999999999999912345678901234567]}");
788+
cJSON *valid_big_number_json_object2 = cJSON_Parse("{\"a\": true, \"b\": [ null,999999999999999999999999999999999999999999999991234567890.1234567E3]}");
789+
const char *invalid_big_number_json1 = "{\"a\": true, \"b\": [ null,99999999999999999999999999999999999999999999999.1234567890.1234567]}";
790+
const char *invalid_big_number_json2 = "{\"a\": true, \"b\": [ null,99999999999999999999999999999999999999999999999E1234567890e1234567]}";
791+
792+
TEST_ASSERT_NOT_NULL(valid_big_number_json_object1);
793+
TEST_ASSERT_NOT_NULL(valid_big_number_json_object2);
794+
TEST_ASSERT_NULL_MESSAGE(cJSON_Parse(invalid_big_number_json1), "Invalid big number JSONs should not be parsed.");
795+
TEST_ASSERT_NULL_MESSAGE(cJSON_Parse(invalid_big_number_json2), "Invalid big number JSONs should not be parsed.");
796+
797+
cJSON_Delete(valid_big_number_json_object1);
798+
cJSON_Delete(valid_big_number_json_object2);
799+
}
800+
785801
int CJSON_CDECL main(void)
786802
{
787803
UNITY_BEGIN();
@@ -815,6 +831,7 @@ int CJSON_CDECL main(void)
815831
RUN_TEST(cjson_delete_item_from_array_should_not_broken_list_structure);
816832
RUN_TEST(cjson_set_valuestring_to_object_should_not_leak_memory);
817833
RUN_TEST(cjson_set_bool_value_must_not_break_objects);
834+
RUN_TEST(cjson_parse_big_numbers_should_not_report_error);
818835

819836
return UNITY_END();
820837
}

tests/parse_number.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,25 @@ static void assert_parse_number(const char *string, int integer, double real)
4848
parse_buffer buffer = { 0, 0, 0, 0, { 0, 0, 0 } };
4949
buffer.content = (const unsigned char*)string;
5050
buffer.length = strlen(string) + sizeof("");
51+
buffer.hooks = global_hooks;
5152

5253
TEST_ASSERT_TRUE(parse_number(item, &buffer));
5354
assert_is_number(item);
5455
TEST_ASSERT_EQUAL_INT(integer, item->valueint);
5556
TEST_ASSERT_EQUAL_DOUBLE(real, item->valuedouble);
5657
}
5758

59+
static void assert_parse_big_number(const char *string)
60+
{
61+
parse_buffer buffer = { 0, 0, 0, 0, { 0, 0, 0 } };
62+
buffer.content = (const unsigned char*)string;
63+
buffer.length = strlen(string) + sizeof("");
64+
buffer.hooks = global_hooks;
65+
66+
TEST_ASSERT_TRUE(parse_number(item, &buffer));
67+
assert_is_number(item);
68+
}
69+
5870
static void parse_number_should_parse_zero(void)
5971
{
6072
assert_parse_number("0", 0, 0);
@@ -96,6 +108,13 @@ static void parse_number_should_parse_negative_reals(void)
96108
assert_parse_number("-123e-128", 0, -123e-128);
97109
}
98110

111+
static void parse_number_should_parse_big_numbers(void)
112+
{
113+
assert_parse_big_number("9999999999999999999999999999999999999999999999912345678901234567");
114+
assert_parse_big_number("9999999999999999999999999999999999999999999999912345678901234567E10");
115+
assert_parse_big_number("999999999999999999999999999999999999999999999991234567890.1234567");
116+
}
117+
99118
int CJSON_CDECL main(void)
100119
{
101120
/* initialize cJSON item */
@@ -106,5 +125,6 @@ int CJSON_CDECL main(void)
106125
RUN_TEST(parse_number_should_parse_positive_integers);
107126
RUN_TEST(parse_number_should_parse_positive_reals);
108127
RUN_TEST(parse_number_should_parse_negative_reals);
128+
RUN_TEST(parse_number_should_parse_big_numbers);
109129
return UNITY_END();
110130
}

0 commit comments

Comments
 (0)