From bf11ca902514969838419d46f334c915d2b58b40 Mon Sep 17 00:00:00 2001 From: Rhodri James Date: Fri, 28 Oct 2022 20:00:24 +0100 Subject: [PATCH] Move (some) XML_GetBuffer tests out of runtests.c --- expat/tests/basic_tests.c | 217 +++++++++++++++++++++++++++++++++ expat/tests/common.c | 29 +++++ expat/tests/common.h | 1 + expat/tests/runtests.c | 246 -------------------------------------- 4 files changed, 247 insertions(+), 246 deletions(-) diff --git a/expat/tests/basic_tests.c b/expat/tests/basic_tests.c index 73cad2a7..efc335ec 100644 --- a/expat/tests/basic_tests.c +++ b/expat/tests/basic_tests.c @@ -2549,6 +2549,214 @@ START_TEST(test_user_parameters) { } END_TEST +/* Test that an explicit external entity handler argument replaces + * the parser as the first argument. + * + * We do not call the first parameter to the external entity handler + * 'parser' for once, since the first time the handler is called it + * will actually be a text string. We need to be able to access the + * global 'parser' variable to create our external entity parser from, + * since there are code paths we need to ensure get executed. + */ +START_TEST(test_ext_entity_ref_parameter) { + const char *text = "\n" + "\n" + "&entity;"; + + XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS); + XML_SetExternalEntityRefHandler(g_parser, external_entity_ref_param_checker); + /* Set a handler arg that is not NULL and not parser (which is + * what NULL would cause to be passed. + */ + XML_SetExternalEntityRefHandlerArg(g_parser, (void *)text); + g_handler_data = (void *)text; + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + + /* Now try again with unset args */ + XML_ParserReset(g_parser, NULL); + XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS); + XML_SetExternalEntityRefHandler(g_parser, external_entity_ref_param_checker); + XML_SetExternalEntityRefHandlerArg(g_parser, NULL); + g_handler_data = (void *)g_parser; + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) + == XML_STATUS_ERROR) + xml_failure(g_parser); +} +END_TEST + +/* Test the parsing of an empty string */ +START_TEST(test_empty_parse) { + const char *text = ""; + const char *partial = ""; + + if (XML_Parse(g_parser, NULL, 0, XML_FALSE) == XML_STATUS_ERROR) + fail("Parsing empty string faulted"); + if (XML_Parse(g_parser, NULL, 0, XML_TRUE) != XML_STATUS_ERROR) + fail("Parsing final empty string not faulted"); + if (XML_GetErrorCode(g_parser) != XML_ERROR_NO_ELEMENTS) + fail("Parsing final empty string faulted for wrong reason"); + + /* Now try with valid text before the empty end */ + XML_ParserReset(g_parser, NULL); + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_FALSE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + if (XML_Parse(g_parser, NULL, 0, XML_TRUE) == XML_STATUS_ERROR) + fail("Parsing final empty string faulted"); + + /* Now try with invalid text before the empty end */ + XML_ParserReset(g_parser, NULL); + if (_XML_Parse_SINGLE_BYTES(g_parser, partial, (int)strlen(partial), + XML_FALSE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + if (XML_Parse(g_parser, NULL, 0, XML_TRUE) != XML_STATUS_ERROR) + fail("Parsing final incomplete empty string not faulted"); +} +END_TEST + +/* Test odd corners of the XML_GetBuffer interface */ +static enum XML_Status +get_feature(enum XML_FeatureEnum feature_id, long *presult) { + const XML_Feature *feature = XML_GetFeatureList(); + + if (feature == NULL) + return XML_STATUS_ERROR; + for (; feature->feature != XML_FEATURE_END; feature++) { + if (feature->feature == feature_id) { + *presult = feature->value; + return XML_STATUS_OK; + } + } + return XML_STATUS_ERROR; +} + +/* Test odd corners of the XML_GetBuffer interface */ +START_TEST(test_get_buffer_1) { + const char *text = get_buffer_test_text; + void *buffer; + long context_bytes; + + /* Attempt to allocate a negative length buffer */ + if (XML_GetBuffer(g_parser, -12) != NULL) + fail("Negative length buffer not failed"); + + /* Now get a small buffer and extend it past valid length */ + buffer = XML_GetBuffer(g_parser, 1536); + if (buffer == NULL) + fail("1.5K buffer failed"); + assert(buffer != NULL); + memcpy(buffer, text, strlen(text)); + if (XML_ParseBuffer(g_parser, (int)strlen(text), XML_FALSE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + if (XML_GetBuffer(g_parser, INT_MAX) != NULL) + fail("INT_MAX buffer not failed"); + + /* Now try extending it a more reasonable but still too large + * amount. The allocator in XML_GetBuffer() doubles the buffer + * size until it exceeds the requested amount or INT_MAX. If it + * exceeds INT_MAX, it rejects the request, so we want a request + * between INT_MAX and INT_MAX/2. A gap of 1K seems comfortable, + * with an extra byte just to ensure that the request is off any + * boundary. The request will be inflated internally by + * XML_CONTEXT_BYTES (if defined), so we subtract that from our + * request. + */ + if (get_feature(XML_FEATURE_CONTEXT_BYTES, &context_bytes) != XML_STATUS_OK) + context_bytes = 0; + if (XML_GetBuffer(g_parser, INT_MAX - (context_bytes + 1025)) != NULL) + fail("INT_MAX- buffer not failed"); + + /* Now try extending it a carefully crafted amount */ + if (XML_GetBuffer(g_parser, 1000) == NULL) + fail("1000 buffer failed"); +} +END_TEST + +/* Test more corners of the XML_GetBuffer interface */ +START_TEST(test_get_buffer_2) { + const char *text = get_buffer_test_text; + void *buffer; + + /* Now get a decent buffer */ + buffer = XML_GetBuffer(g_parser, 1536); + if (buffer == NULL) + fail("1.5K buffer failed"); + assert(buffer != NULL); + memcpy(buffer, text, strlen(text)); + if (XML_ParseBuffer(g_parser, (int)strlen(text), XML_FALSE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + + /* Extend it, to catch a different code path */ + if (XML_GetBuffer(g_parser, 1024) == NULL) + fail("1024 buffer failed"); +} +END_TEST + +/* Test for signed integer overflow CVE-2022-23852 */ +#if defined(XML_CONTEXT_BYTES) +START_TEST(test_get_buffer_3_overflow) { + XML_Parser parser = XML_ParserCreate(NULL); + assert(parser != NULL); + + const char *const text = "\n"; + const int expectedKeepValue = (int)strlen(text); + + // After this call, variable "keep" in XML_GetBuffer will + // have value expectedKeepValue + if (XML_Parse(parser, text, (int)strlen(text), XML_FALSE /* isFinal */) + == XML_STATUS_ERROR) + xml_failure(parser); + + assert(expectedKeepValue > 0); + if (XML_GetBuffer(parser, INT_MAX - expectedKeepValue + 1) != NULL) + fail("enlarging buffer not failed"); + + XML_ParserFree(parser); +} +END_TEST +#endif // defined(XML_CONTEXT_BYTES) + +/* Test position information macros */ +START_TEST(test_byte_info_at_end) { + const char *text = ""; + + if (XML_GetCurrentByteIndex(g_parser) != -1 + || XML_GetCurrentByteCount(g_parser) != 0) + fail("Byte index/count incorrect at start of parse"); + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) + == XML_STATUS_ERROR) + xml_failure(g_parser); + /* At end, the count will be zero and the index the end of string */ + if (XML_GetCurrentByteCount(g_parser) != 0) + fail("Terminal byte count incorrect"); + if (XML_GetCurrentByteIndex(g_parser) != (XML_Index)strlen(text)) + fail("Terminal byte index incorrect"); +} +END_TEST + +/* Test position information from errors */ +#define PRE_ERROR_STR "" +START_TEST(test_byte_info_at_error) { + const char *text = PRE_ERROR_STR POST_ERROR_STR; + + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) + == XML_STATUS_OK) + fail("Syntax error not faulted"); + if (XML_GetCurrentByteCount(g_parser) != 0) + fail("Error byte count incorrect"); + if (XML_GetCurrentByteIndex(g_parser) != strlen(PRE_ERROR_STR)) + fail("Error byte index incorrect"); +} +END_TEST +#undef PRE_ERROR_STR +#undef POST_ERROR_STR + TCase * make_basic_test_case(Suite *s) { TCase *tc_basic = tcase_create("basic tests"); @@ -2662,6 +2870,15 @@ make_basic_test_case(Suite *s) { tcase_add_test(tc_basic, test_ext_entity_trailing_rsqb); tcase_add_test(tc_basic, test_ext_entity_good_cdata); tcase_add_test__ifdef_xml_dtd(tc_basic, test_user_parameters); + tcase_add_test__ifdef_xml_dtd(tc_basic, test_ext_entity_ref_parameter); + tcase_add_test(tc_basic, test_empty_parse); + tcase_add_test(tc_basic, test_get_buffer_1); + tcase_add_test(tc_basic, test_get_buffer_2); +#if defined(XML_CONTEXT_BYTES) + tcase_add_test(tc_basic, test_get_buffer_3_overflow); +#endif + tcase_add_test(tc_basic, test_byte_info_at_end); + tcase_add_test(tc_basic, test_byte_info_at_error); return tc_basic; /* TEMPORARY: this will become a void function */ } diff --git a/expat/tests/common.c b/expat/tests/common.c index 99d1f79f..23be8bab 100644 --- a/expat/tests/common.c +++ b/expat/tests/common.c @@ -100,6 +100,35 @@ const char *long_cdata_text "012345678901234567890123456789012345678901234567890123456789" "]]>"; +/* Having an element name longer than 1024 characters exercises some + * of the pool allocation code in the parser that otherwise does not + * get executed. The count at the end of the line is the number of + * characters (bytes) in the element name by that point.x + */ +const char *get_buffer_test_text + = "\n\n" - "&entity;"; - - XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS); - XML_SetExternalEntityRefHandler(g_parser, external_entity_ref_param_checker); - /* Set a handler arg that is not NULL and not parser (which is - * what NULL would cause to be passed. - */ - XML_SetExternalEntityRefHandlerArg(g_parser, (void *)text); - g_handler_data = (void *)text; - if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) - == XML_STATUS_ERROR) - xml_failure(g_parser); - - /* Now try again with unset args */ - XML_ParserReset(g_parser, NULL); - XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS); - XML_SetExternalEntityRefHandler(g_parser, external_entity_ref_param_checker); - XML_SetExternalEntityRefHandlerArg(g_parser, NULL); - g_handler_data = (void *)g_parser; - if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) - == XML_STATUS_ERROR) - xml_failure(g_parser); -} -END_TEST - -/* Test the parsing of an empty string */ -START_TEST(test_empty_parse) { - const char *text = ""; - const char *partial = ""; - - if (XML_Parse(g_parser, NULL, 0, XML_FALSE) == XML_STATUS_ERROR) - fail("Parsing empty string faulted"); - if (XML_Parse(g_parser, NULL, 0, XML_TRUE) != XML_STATUS_ERROR) - fail("Parsing final empty string not faulted"); - if (XML_GetErrorCode(g_parser) != XML_ERROR_NO_ELEMENTS) - fail("Parsing final empty string faulted for wrong reason"); - - /* Now try with valid text before the empty end */ - XML_ParserReset(g_parser, NULL); - if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_FALSE) - == XML_STATUS_ERROR) - xml_failure(g_parser); - if (XML_Parse(g_parser, NULL, 0, XML_TRUE) == XML_STATUS_ERROR) - fail("Parsing final empty string faulted"); - - /* Now try with invalid text before the empty end */ - XML_ParserReset(g_parser, NULL); - if (_XML_Parse_SINGLE_BYTES(g_parser, partial, (int)strlen(partial), - XML_FALSE) - == XML_STATUS_ERROR) - xml_failure(g_parser); - if (XML_Parse(g_parser, NULL, 0, XML_TRUE) != XML_STATUS_ERROR) - fail("Parsing final incomplete empty string not faulted"); -} -END_TEST - -/* Test odd corners of the XML_GetBuffer interface */ -static enum XML_Status -get_feature(enum XML_FeatureEnum feature_id, long *presult) { - const XML_Feature *feature = XML_GetFeatureList(); - - if (feature == NULL) - return XML_STATUS_ERROR; - for (; feature->feature != XML_FEATURE_END; feature++) { - if (feature->feature == feature_id) { - *presult = feature->value; - return XML_STATUS_OK; - } - } - return XML_STATUS_ERROR; -} - -/* Having an element name longer than 1024 characters exercises some - * of the pool allocation code in the parser that otherwise does not - * get executed. The count at the end of the line is the number of - * characters (bytes) in the element name by that point.x - */ -static const char *get_buffer_test_text - = "\n 0); - if (XML_GetBuffer(parser, INT_MAX - expectedKeepValue + 1) != NULL) - fail("enlarging buffer not failed"); - - XML_ParserFree(parser); -} -END_TEST -#endif // defined(XML_CONTEXT_BYTES) - -/* Test position information macros */ -START_TEST(test_byte_info_at_end) { - const char *text = ""; - - if (XML_GetCurrentByteIndex(g_parser) != -1 - || XML_GetCurrentByteCount(g_parser) != 0) - fail("Byte index/count incorrect at start of parse"); - if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) - == XML_STATUS_ERROR) - xml_failure(g_parser); - /* At end, the count will be zero and the index the end of string */ - if (XML_GetCurrentByteCount(g_parser) != 0) - fail("Terminal byte count incorrect"); - if (XML_GetCurrentByteIndex(g_parser) != (XML_Index)strlen(text)) - fail("Terminal byte index incorrect"); -} -END_TEST - -/* Test position information from errors */ -#define PRE_ERROR_STR "" -START_TEST(test_byte_info_at_error) { - const char *text = PRE_ERROR_STR POST_ERROR_STR; - - if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) - == XML_STATUS_OK) - fail("Syntax error not faulted"); - if (XML_GetCurrentByteCount(g_parser) != 0) - fail("Error byte count incorrect"); - if (XML_GetCurrentByteIndex(g_parser) != strlen(PRE_ERROR_STR)) - fail("Error byte index incorrect"); -} -END_TEST -#undef PRE_ERROR_STR -#undef POST_ERROR_STR - /* Test position information in handler */ typedef struct ByteTestData { int start_element_len; @@ -8394,15 +8157,6 @@ make_suite(void) { TCase *tc_accounting = tcase_create("accounting tests"); #endif - tcase_add_test__ifdef_xml_dtd(tc_basic, test_ext_entity_ref_parameter); - tcase_add_test(tc_basic, test_empty_parse); - tcase_add_test(tc_basic, test_get_buffer_1); - tcase_add_test(tc_basic, test_get_buffer_2); -#if defined(XML_CONTEXT_BYTES) - tcase_add_test(tc_basic, test_get_buffer_3_overflow); -#endif - tcase_add_test(tc_basic, test_byte_info_at_end); - tcase_add_test(tc_basic, test_byte_info_at_error); tcase_add_test(tc_basic, test_byte_info_at_cdata); tcase_add_test(tc_basic, test_predefined_entities); tcase_add_test__ifdef_xml_dtd(tc_basic, test_invalid_tag_in_dtd);