From 5290462a7ea1278a8d5c0d5b2860d4e244f997e4 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Tue, 20 Sep 2022 02:44:34 +0200 Subject: [PATCH 1/3] lib: Fix overeager DTD destruction in XML_ExternalEntityParserCreate --- lib/xmlparse.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/xmlparse.c b/lib/xmlparse.c index aacd6e7fc..57bf103cc 100644 --- a/lib/xmlparse.c +++ b/lib/xmlparse.c @@ -1068,6 +1068,14 @@ parserCreate(const XML_Char *encodingName, parserInit(parser, encodingName); if (encodingName && ! parser->m_protocolEncodingName) { + if (dtd) { + // We need to stop the upcoming call to XML_ParserFree from happily + // destroying parser->m_dtd because the DTD is shared with the parent + // parser and the only guard that keeps XML_ParserFree from destroying + // parser->m_dtd is parser->m_isParamEntity but it will be set to + // XML_TRUE only later in XML_ExternalEntityParserCreate (or not at all). + parser->m_dtd = NULL; + } XML_ParserFree(parser); return NULL; } From 43992e4ae25fc3dc0eec0cd3a29313555d56aee2 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Mon, 19 Sep 2022 18:16:15 +0200 Subject: [PATCH 2/3] tests: Cover overeager DTD destruction in XML_ExternalEntityParserCreate --- tests/runtests.c | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/runtests.c b/tests/runtests.c index 245fe9bda..acb744dd4 100644 --- a/tests/runtests.c +++ b/tests/runtests.c @@ -10208,6 +10208,53 @@ START_TEST(test_alloc_long_notation) { } END_TEST +static int XMLCALL +external_entity_parser_create_alloc_fail_handler(XML_Parser parser, + const XML_Char *context, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId) { + UNUSED_P(base); + UNUSED_P(systemId); + UNUSED_P(publicId); + + if (context != NULL) + fail("Unexpected non-NULL context"); + + // The following number intends to fail the upcoming allocation in line + // "parser->m_protocolEncodingName = copyString(encodingName, + // &(parser->m_mem));" in function parserInit. + allocation_count = 3; + + const XML_Char *const encodingName = XCS("UTF-8"); // needs something non-NULL + const XML_Parser ext_parser + = XML_ExternalEntityParserCreate(parser, context, encodingName); + if (ext_parser != NULL) + fail( + "Call to XML_ExternalEntityParserCreate was expected to fail out-of-memory"); + + allocation_count = ALLOC_ALWAYS_SUCCEED; + return XML_STATUS_ERROR; +} + +START_TEST(test_alloc_reset_after_external_entity_parser_create_fail) { + const char *const text = ""; + + XML_SetExternalEntityRefHandler( + 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) + != XML_STATUS_ERROR) + fail("Call to parse was expected to fail"); + + if (XML_GetErrorCode(g_parser) != XML_ERROR_EXTERNAL_ENTITY_HANDLING) + fail("Call to parse was expected to fail from the external entity handler"); + + XML_ParserReset(g_parser, NULL); +} +END_TEST + static void nsalloc_setup(void) { XML_Memory_Handling_Suite memsuite = {duff_allocator, duff_reallocator, free}; @@ -12401,6 +12448,8 @@ make_suite(void) { tcase_add_test(tc_alloc, test_alloc_long_public_id); tcase_add_test(tc_alloc, test_alloc_long_entity_value); tcase_add_test(tc_alloc, test_alloc_long_notation); + tcase_add_test__ifdef_xml_dtd( + tc_alloc, test_alloc_reset_after_external_entity_parser_create_fail); suite_add_tcase(s, tc_nsalloc); tcase_add_checked_fixture(tc_nsalloc, nsalloc_setup, nsalloc_teardown);