From 795cf9992248b00be33cbde922e0b1ecc32a3491 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Thu, 9 Nov 2023 20:18:24 +0100 Subject: [PATCH 1/3] tests: Make _XML_Parse_SINGLE_BYTES deny use with pathological input --- expat/tests/common.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/expat/tests/common.c b/expat/tests/common.c index 2832933a..9fe4c8be 100644 --- a/expat/tests/common.c +++ b/expat/tests/common.c @@ -41,6 +41,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include #include #include @@ -190,6 +191,10 @@ _xml_failure(XML_Parser parser, const char *file, int line) { enum XML_Status _XML_Parse_SINGLE_BYTES(XML_Parser parser, const char *s, int len, int isFinal) { + // This ensures that tests have to run pathological parse cases + // (e.g. when `s` is NULL) against plain XML_Parse rather than + // chunking _XML_Parse_SINGLE_BYTES. + assert((parser != NULL) && (s != NULL) && (len >= 0)); const int chunksize = g_chunkSize; int offset = 0; if (chunksize > 0) { From 3e5f6d66017d82a4070f22eb8a9ed8a9cdbd24a3 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Tue, 7 Nov 2023 15:39:00 +0100 Subject: [PATCH 2/3] tests: Migrate more tests to variable chunk size parsing --- expat/tests/alloc_tests.c | 2 +- expat/tests/basic_tests.c | 34 +++++++++++++++++++--------------- expat/tests/handlers.c | 6 +++--- expat/tests/misc_tests.c | 14 ++++++++------ expat/tests/ns_tests.c | 4 ++-- 5 files changed, 33 insertions(+), 27 deletions(-) diff --git a/expat/tests/alloc_tests.c b/expat/tests/alloc_tests.c index 129f3e71..4f47fc8a 100644 --- a/expat/tests/alloc_tests.c +++ b/expat/tests/alloc_tests.c @@ -2048,7 +2048,7 @@ START_TEST(test_alloc_reset_after_external_entity_parser_create_fail) { g_parser, external_entity_parser_create_alloc_fail_handler); XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) fail("Call to parse was expected to fail"); diff --git a/expat/tests/basic_tests.c b/expat/tests/basic_tests.c index 949cc4a2..02733a3e 100644 --- a/expat/tests/basic_tests.c +++ b/expat/tests/basic_tests.c @@ -1379,7 +1379,7 @@ START_TEST(test_suspend_parser_between_char_data_calls) { if (XML_GetErrorCode(g_parser) != XML_ERROR_NONE) xml_failure(g_parser); /* Try parsing directly */ - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) fail("Attempt to continue parse while suspended not faulted"); if (XML_GetErrorCode(g_parser) != XML_ERROR_SUSPENDED) @@ -2130,7 +2130,7 @@ START_TEST(test_dtd_elements_nesting) { XML_SetUserData(g_parser, (void *)(uintptr_t)-1); XML_SetElementDeclHandler(g_parser, element_decl_check_model); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) == XML_STATUS_ERROR) xml_failure(g_parser); @@ -2848,7 +2848,8 @@ START_TEST(test_get_buffer_3_overflow) { // After this call, variable "keep" in XML_GetBuffer will // have value expectedKeepValue - if (XML_Parse(parser, text, (int)strlen(text), XML_FALSE /* isFinal */) + if (_XML_Parse_SINGLE_BYTES(parser, text, (int)strlen(text), + XML_FALSE /* isFinal */) == XML_STATUS_ERROR) xml_failure(parser); @@ -3160,7 +3161,8 @@ external_bom_checker(XML_Parser parser, const XML_Char *context, const int split = testdata->split; testdata->nested_callback_happened = XML_TRUE; - if (XML_Parse(ext_parser, external, split, XML_FALSE) != XML_STATUS_OK) { + if (_XML_Parse_SINGLE_BYTES(ext_parser, external, split, XML_FALSE) + != XML_STATUS_OK) { xml_failure(ext_parser); } text = external + split; // the parse below will continue where we left off. @@ -3172,7 +3174,8 @@ external_bom_checker(XML_Parser parser, const XML_Char *context, fail("unknown systemId"); } - if (XML_Parse(ext_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_OK) + if (_XML_Parse_SINGLE_BYTES(ext_parser, text, (int)strlen(text), XML_TRUE) + != XML_STATUS_OK) xml_failure(ext_parser); XML_ParserFree(ext_parser); @@ -3512,7 +3515,7 @@ START_TEST(test_suspend_xdecl) { if (XML_GetErrorCode(g_parser) != XML_ERROR_NONE) xml_failure(g_parser); /* Attempt to start a new parse while suspended */ - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) fail("Attempt to parse while suspended not faulted"); if (XML_GetErrorCode(g_parser) != XML_ERROR_SUSPENDED) @@ -3619,7 +3622,7 @@ START_TEST(test_suspend_resume_internal_entity) { XML_SetStartElementHandler(g_parser, start_element_suspender); XML_SetCharacterDataHandler(g_parser, accumulate_characters); XML_SetUserData(g_parser, &storage); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_SUSPENDED) xml_failure(g_parser); CharData_CheckXMLChars(&storage, XCS("")); @@ -3689,8 +3692,9 @@ START_TEST(test_suspend_resume_internal_entity_issue_629) { xml_failure(parser); if (XML_ResumeParser(parser) != XML_STATUS_OK) xml_failure(parser); - if (XML_Parse(parser, text + firstChunkSizeBytes, - (int)(strlen(text) - firstChunkSizeBytes), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(parser, text + firstChunkSizeBytes, + (int)(strlen(text) - firstChunkSizeBytes), + XML_TRUE) != XML_STATUS_OK) xml_failure(parser); XML_ParserFree(parser); @@ -3705,7 +3709,7 @@ START_TEST(test_resume_entity_with_syntax_error) { "&foo;\n"; XML_SetStartElementHandler(g_parser, start_element_suspender); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_SUSPENDED) xml_failure(g_parser); if (XML_ResumeParser(g_parser) != XML_STATUS_ERROR) @@ -3744,7 +3748,7 @@ END_TEST START_TEST(test_restart_on_error) { const char *text = "<$doc>"; - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) fail("Invalid tag name not faulted"); if (XML_GetErrorCode(g_parser) != XML_ERROR_INVALID_TOKEN) @@ -4398,7 +4402,7 @@ START_TEST(test_ext_entity_latin1_utf16le_bom2) { XML_SetExternalEntityRefHandler(g_parser, external_entity_loader2); XML_SetUserData(g_parser, &test_data); XML_SetCharacterDataHandler(g_parser, ext2_accumulate_characters); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) == XML_STATUS_ERROR) xml_failure(g_parser); CharData_CheckXMLChars(&storage, expected); @@ -4429,7 +4433,7 @@ START_TEST(test_ext_entity_latin1_utf16be_bom2) { XML_SetExternalEntityRefHandler(g_parser, external_entity_loader2); XML_SetUserData(g_parser, &test_data); XML_SetCharacterDataHandler(g_parser, ext2_accumulate_characters); - if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE) == XML_STATUS_ERROR) xml_failure(g_parser); CharData_CheckXMLChars(&storage, expected); @@ -4640,8 +4644,8 @@ START_TEST(test_utf8_in_start_tags) { cases[i].tagName); XML_Parser parser = XML_ParserCreate(NULL); - const enum XML_Status status - = XML_Parse(parser, doc, (int)strlen(doc), /*isFinal=*/XML_FALSE); + const enum XML_Status status = _XML_Parse_SINGLE_BYTES( + parser, doc, (int)strlen(doc), /*isFinal=*/XML_FALSE); bool success = true; if ((status == XML_STATUS_OK) != expectedSuccess) { diff --git a/expat/tests/handlers.c b/expat/tests/handlers.c index 1fb65ed1..8c7d8488 100644 --- a/expat/tests/handlers.c +++ b/expat/tests/handlers.c @@ -530,7 +530,7 @@ external_entity_resetter(XML_Parser parser, const XML_Char *context, return XML_STATUS_ERROR; } /* Check we can't parse here */ - if (XML_Parse(ext_parser, text, (int)strlen(text), XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(ext_parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) fail("Parsing when finished not faulted"); if (XML_GetErrorCode(ext_parser) != XML_ERROR_FINISHED) @@ -1192,8 +1192,8 @@ external_entity_faulter2(XML_Parser parser, const XML_Char *context, if (! XML_SetEncoding(extparser, test_data->encoding)) fail("XML_SetEncoding() ignored for external entity"); } - if (XML_Parse(extparser, test_data->parse_text, test_data->parse_len, - XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(extparser, test_data->parse_text, + test_data->parse_len, XML_TRUE) != XML_STATUS_ERROR) fail(test_data->fail_text); if (XML_GetErrorCode(extparser) != test_data->error) diff --git a/expat/tests/misc_tests.c b/expat/tests/misc_tests.c index 4167c4cc..029572d1 100644 --- a/expat/tests/misc_tests.c +++ b/expat/tests/misc_tests.c @@ -276,7 +276,7 @@ START_TEST(test_misc_stop_during_end_handler_issue_240_1) { mydata->deep = 0; XML_SetUserData(parser, mydata); - result = XML_Parse(parser, doc1, (int)strlen(doc1), 1); + result = _XML_Parse_SINGLE_BYTES(parser, doc1, (int)strlen(doc1), 1); XML_ParserFree(parser); free(mydata); if (result != XML_STATUS_ERROR) @@ -297,7 +297,7 @@ START_TEST(test_misc_stop_during_end_handler_issue_240_2) { mydata->deep = 0; XML_SetUserData(parser, mydata); - result = XML_Parse(parser, doc2, (int)strlen(doc2), 1); + result = _XML_Parse_SINGLE_BYTES(parser, doc2, (int)strlen(doc2), 1); XML_ParserFree(parser); free(mydata); if (result != XML_STATUS_ERROR) @@ -343,9 +343,9 @@ START_TEST(test_misc_deny_internal_entity_closing_doctype_issue_317) { if (setParamEntityResult != 1) fail("Failed to set XML_PARAM_ENTITY_PARSING_ALWAYS."); - parseResult = XML_Parse(parser, input, (int)strlen(input), 0); + parseResult = _XML_Parse_SINGLE_BYTES(parser, input, (int)strlen(input), 0); if (parseResult != XML_STATUS_ERROR) { - parseResult = XML_Parse(parser, "", 0, 1); + parseResult = _XML_Parse_SINGLE_BYTES(parser, "", 0, 1); if (parseResult != XML_STATUS_ERROR) { fail("Parsing was expected to fail but succeeded."); } @@ -372,14 +372,16 @@ START_TEST(test_misc_tag_mismatch_reset_leak) { const char *const text = ""; XML_Parser parser = XML_ParserCreateNS(NULL, XCS('\n')); - if (XML_Parse(parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) + if (_XML_Parse_SINGLE_BYTES(parser, text, (int)strlen(text), XML_TRUE) + != XML_STATUS_ERROR) fail("Call to parse was expected to fail"); if (XML_GetErrorCode(parser) != XML_ERROR_TAG_MISMATCH) fail("Call to parse was expected to fail from a closing tag mismatch"); XML_ParserReset(parser, NULL); - if (XML_Parse(parser, text, (int)strlen(text), XML_TRUE) != XML_STATUS_ERROR) + if (_XML_Parse_SINGLE_BYTES(parser, text, (int)strlen(text), XML_TRUE) + != XML_STATUS_ERROR) fail("Call to parse was expected to fail"); if (XML_GetErrorCode(parser) != XML_ERROR_TAG_MISMATCH) fail("Call to parse was expected to fail from a closing tag mismatch"); diff --git a/expat/tests/ns_tests.c b/expat/tests/ns_tests.c index 34683def..84e5829b 100644 --- a/expat/tests/ns_tests.c +++ b/expat/tests/ns_tests.c @@ -697,8 +697,8 @@ START_TEST(test_ns_separator_in_uri) { set_subtest("%s", cases[i].doc); XML_Parser parser = XML_ParserCreateNS(NULL, cases[i].namesep); XML_SetElementHandler(parser, dummy_start_element, dummy_end_element); - if (XML_Parse(parser, cases[i].doc, (int)strlen(cases[i].doc), - /*isFinal*/ XML_TRUE) + if (_XML_Parse_SINGLE_BYTES(parser, cases[i].doc, (int)strlen(cases[i].doc), + /*isFinal*/ XML_TRUE) != cases[i].expectedStatus) { failCount++; } From d2b31760cd5d22b26316d407789caded826857e3 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Tue, 7 Nov 2023 15:55:03 +0100 Subject: [PATCH 3/3] tests: Simplify _XML_Parse_SINGLE_BYTES Co-authored-by: Snild Dolkow --- expat/tests/common.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/expat/tests/common.c b/expat/tests/common.c index 9fe4c8be..4a552e60 100644 --- a/expat/tests/common.c +++ b/expat/tests/common.c @@ -196,18 +196,17 @@ _XML_Parse_SINGLE_BYTES(XML_Parser parser, const char *s, int len, // chunking _XML_Parse_SINGLE_BYTES. assert((parser != NULL) && (s != NULL) && (len >= 0)); const int chunksize = g_chunkSize; - int offset = 0; if (chunksize > 0) { - // parse in chunks of `chunksize` bytes as long as possible - for (; offset + chunksize < len; offset += chunksize) { - enum XML_Status res = XML_Parse(parser, s + offset, chunksize, XML_FALSE); + // parse in chunks of `chunksize` bytes as long as not exhausting + for (; len > chunksize; len -= chunksize, s += chunksize) { + enum XML_Status res = XML_Parse(parser, s, chunksize, XML_FALSE); if (res != XML_STATUS_OK) { return res; } } } // parse the final chunk, the size of which will be <= chunksize - return XML_Parse(parser, s + offset, len - offset, isFinal); + return XML_Parse(parser, s, len, isFinal); } void