From 774ee6c15b8832ff567d2a32ca0e4926179e7f97 Mon Sep 17 00:00:00 2001 From: Rhodri James Date: Wed, 14 Jun 2017 23:44:45 +0200 Subject: [PATCH 1/3] Tests: Cover external entity infinite loop bug --- expat/tests/runtests.c | 66 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/expat/tests/runtests.c b/expat/tests/runtests.c index 55d9a4c3..857f77f8 100644 --- a/expat/tests/runtests.c +++ b/expat/tests/runtests.c @@ -2287,6 +2287,71 @@ START_TEST(test_byte_info_at_cdata) } END_TEST +/* Regression test that an invalid tag in an external parameter + * reference in an external DTD is correctly faulted. + * + * Only a few specific tags are legal in DTDs ignoring comments and + * processing instructions, all of which begin with an exclamation + * mark. "" is not one of them, so the parser should raise an + * error on encountering it. + */ +static int XMLCALL +external_entity_param(XML_Parser parser, + const XML_Char *context, + const XML_Char *UNUSED_P(base), + const XML_Char *systemId, + const XML_Char *UNUSED_P(publicId)) +{ + const char *text1 = + "\n" + "\n" + "\n" + "%e1;\n"; + const char *text2 = + "\n" + "\n"; + XML_Parser ext_parser; + + if (systemId == NULL) + return XML_STATUS_OK; + + ext_parser = XML_ExternalEntityParserCreate(parser, context, NULL); + if (ext_parser == NULL) + fail("Could not create external entity parser"); + + if (!strcmp(systemId, "004-1.ent")) { + if (_XML_Parse_SINGLE_BYTES(ext_parser, text1, strlen(text1), + XML_TRUE) != XML_STATUS_ERROR) + fail("Inner DTD with invalid tag not rejected"); + if (XML_GetErrorCode(ext_parser) != XML_ERROR_EXTERNAL_ENTITY_HANDLING) + xml_failure(ext_parser); + } + else if (!strcmp(systemId, "004-2.ent")) { + if (_XML_Parse_SINGLE_BYTES(ext_parser, text2, strlen(text2), + XML_TRUE) != XML_STATUS_ERROR) + fail("Invalid tag in external param not rejected"); + if (XML_GetErrorCode(ext_parser) != XML_ERROR_SYNTAX) + xml_failure(ext_parser); + } else { + fail("Unknown system ID"); + } + + return XML_STATUS_ERROR; +} + +START_TEST(test_invalid_tag_in_dtd) +{ + const char *text = + "\n" + "\n"; + + XML_SetParamEntityParsing(parser, XML_PARAM_ENTITY_PARSING_ALWAYS); + XML_SetExternalEntityRefHandler(parser, external_entity_param); + expect_failure(text, XML_ERROR_EXTERNAL_ENTITY_HANDLING, + "Invalid tag IN DTD external param not rejected"); +} +END_TEST + /* * Namespaces tests. @@ -3432,6 +3497,7 @@ make_suite(void) 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_invalid_tag_in_dtd); suite_add_tcase(s, tc_namespace); tcase_add_checked_fixture(tc_namespace, From c4bf96bb51dd2a1b0e185374362ee136fe2c9d7f Mon Sep 17 00:00:00 2001 From: Rhodri James Date: Wed, 14 Jun 2017 23:45:07 +0200 Subject: [PATCH 2/3] xmlparse.c: Fix external entity infinite loop bug (CVE-2017-9233) --- expat/lib/xmlparse.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c index 7818f8df..21145964 100644 --- a/expat/lib/xmlparse.c +++ b/expat/lib/xmlparse.c @@ -3981,6 +3981,14 @@ entityValueInitProcessor(XML_Parser parser, *nextPtr = next; return XML_ERROR_NONE; } + /* If we get this token, we have the start of what might be a + normal tag, but not a declaration (i.e. it doesn't begin with + " Date: Wed, 14 Jun 2017 14:09:58 +0200 Subject: [PATCH 3/3] Changes: Add CVE-2017-9233 info to change log --- expat/Changes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/expat/Changes b/expat/Changes index b48133fd..50c2711b 100644 --- a/expat/Changes +++ b/expat/Changes @@ -4,6 +4,9 @@ NOTE: We are looking for help with a few things: Release 2.2.1 ?????????? Security fixes: + CVE-2017-9233 -- External entity infinite loop DoS + Details: https://libexpat.github.io/doc/cve-2017-9233/ + Commit c4bf96bb51dd2a1b0e185374362ee136fe2c9d7f CVE-2016-9063 -- Detect integer overflow; commit d4f735b88d9932bd5039df2335eefdd0723dbe20 (Fixed version of existing downstream patches!)