Grabbed from Debian since upstream patch does not apply to 2.2.7. https://sources.debian.org/patches/expat/2.2.7-2/CVE-2019-15903_Deny_internal_entities_closing_the_doctype.patch/ https://github.com/libexpat/libexpat/commit/c20b758c332d9a13afbbb276d30db1d183a85d43 From c20b758c332d9a13afbbb276d30db1d183a85d43 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Wed, 28 Aug 2019 00:24:59 +0200 Subject: [PATCH 1/3] xmlparse.c: Deny internal entities closing the doctype diff --git a/lib/xmlparse.c b/lib/xmlparse.c index 0553e3df..c29a6449 100644 --- a/lib/xmlparse.c +++ b/lib/xmlparse.c @@ -405,7 +405,7 @@ initializeEncoding(XML_Parser parser); static enum XML_Error doProlog(XML_Parser parser, const ENCODING *enc, const char *s, const char *end, int tok, const char *next, const char **nextPtr, - XML_Bool haveMore); + XML_Bool haveMore, XML_Bool allowClosingDoctype); static enum XML_Error processInternalEntity(XML_Parser parser, ENTITY *entity, XML_Bool betweenDecl); @@ -4232,7 +4232,7 @@ externalParEntProcessor(XML_Parser parse parser->m_processor = prologProcessor; return doProlog(parser, parser->m_encoding, s, end, tok, next, - nextPtr, (XML_Bool)!parser->m_parsingStatus.finalBuffer); + nextPtr, (XML_Bool)!parser->m_parsingStatus.finalBuffer, XML_TRUE); } static enum XML_Error PTRCALL @@ -4282,7 +4282,7 @@ prologProcessor(XML_Parser parser, const char *next = s; int tok = XmlPrologTok(parser->m_encoding, s, end, &next); return doProlog(parser, parser->m_encoding, s, end, tok, next, - nextPtr, (XML_Bool)!parser->m_parsingStatus.finalBuffer); + nextPtr, (XML_Bool)!parser->m_parsingStatus.finalBuffer, XML_TRUE); } static enum XML_Error @@ -4293,7 +4293,8 @@ doProlog(XML_Parser parser, int tok, const char *next, const char **nextPtr, - XML_Bool haveMore) + XML_Bool haveMore, + XML_Bool allowClosingDoctype) { #ifdef XML_DTD static const XML_Char externalSubsetName[] = { ASCII_HASH , '\0' }; @@ -4472,6 +4473,11 @@ doProlog(XML_Parser parser, } break; case XML_ROLE_DOCTYPE_CLOSE: + if (allowClosingDoctype != XML_TRUE) { + /* Must not close doctype from within expanded parameter entities */ + return XML_ERROR_INVALID_TOKEN; + } + if (parser->m_doctypeName) { parser->m_startDoctypeDeclHandler(parser->m_handlerArg, parser->m_doctypeName, parser->m_doctypeSysid, parser->m_doctypePubid, 0); @@ -5409,7 +5415,7 @@ processInternalEntity(XML_Parser parser, if (entity->is_param) { int tok = XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next); result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd, tok, - next, &next, XML_FALSE); + next, &next, XML_FALSE, XML_FALSE); } else #endif /* XML_DTD */ @@ -5456,7 +5462,7 @@ internalEntityProcessor(XML_Parser parse if (entity->is_param) { int tok = XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next); result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd, tok, - next, &next, XML_FALSE); + next, &next, XML_FALSE, XML_TRUE); } else #endif /* XML_DTD */ @@ -5483,7 +5489,7 @@ internalEntityProcessor(XML_Parser parse parser->m_processor = prologProcessor; tok = XmlPrologTok(parser->m_encoding, s, end, &next); return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr, - (XML_Bool)!parser->m_parsingStatus.finalBuffer); + (XML_Bool)!parser->m_parsingStatus.finalBuffer, XML_TRUE); } else #endif /* XML_DTD */ From 438493691f1b8620a71d5aee658fe160103ff863 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Wed, 28 Aug 2019 15:14:19 +0200 Subject: [PATCH 3/3] tests: Cover denying internal entities closing the doctype diff --git a/tests/runtests.c b/tests/runtests.c index b0d1b0af..e102a55e 100644 --- a/tests/runtests.c +++ b/tests/runtests.c @@ -8151,6 +8151,68 @@ START_TEST(test_misc_utf16le) } END_TEST +#ifdef XML_DTD +START_TEST(test_misc_deny_internal_entity_closing_doctype_issue_317) { + const char *const inputOne = "'>\n" + "\n" + "%e;"; + const char *const inputTwo = "'>\n" + "\n" + "%e2;"; + const char *const inputThree = "\n" + "\n" + "%e;"; + const char *const inputIssue317 = "\n" + "Hell'>\n" + "%foo;\n" + "]>\n" + "Hello, world"; + + const char *const inputs[] = {inputOne, inputTwo, inputThree, inputIssue317}; + size_t inputIndex = 0; + + for (; inputIndex < sizeof(inputs) / sizeof(inputs[0]); inputIndex++) { + XML_Parser parser; + enum XML_Status parseResult; + int setParamEntityResult; + XML_Size lineNumber; + XML_Size columnNumber; + const char *const input = inputs[inputIndex]; + + parser = XML_ParserCreate(NULL); + setParamEntityResult + = XML_SetParamEntityParsing(parser, XML_PARAM_ENTITY_PARSING_ALWAYS); + if (setParamEntityResult != 1) + fail("Failed to set XML_PARAM_ENTITY_PARSING_ALWAYS."); + + parseResult = XML_Parse(parser, input, (int)strlen(input), 0); + if (parseResult != XML_STATUS_ERROR) { + parseResult = XML_Parse(parser, "", 0, 1); + if (parseResult != XML_STATUS_ERROR) { + fail("Parsing was expected to fail but succeeded."); + } + } + + if (XML_GetErrorCode(parser) != XML_ERROR_INVALID_TOKEN) + fail("Error code does not match XML_ERROR_INVALID_TOKEN"); + + lineNumber = XML_GetCurrentLineNumber(parser); + if (lineNumber != 4) + fail("XML_GetCurrentLineNumber does not work as expected."); + + columnNumber = XML_GetCurrentColumnNumber(parser); + if (columnNumber != 0) + fail("XML_GetCurrentColumnNumber does not work as expected."); + + XML_ParserFree(parser); + } +} +END_TEST +#endif static void alloc_setup(void) @@ -12251,6 +12313,10 @@ make_suite(void) tcase_add_test(tc_misc, test_misc_features); tcase_add_test(tc_misc, test_misc_attribute_leak); tcase_add_test(tc_misc, test_misc_utf16le); +#ifdef XML_DTD + tcase_add_test(tc_misc, + test_misc_deny_internal_entity_closing_doctype_issue_317); +#endif suite_add_tcase(s, tc_alloc); tcase_add_checked_fixture(tc_alloc, alloc_setup, alloc_teardown);