From f23fd2fa9c6b46703fb8b0068848da9dd7807900 Mon Sep 17 00:00:00 2001 From: Tomas Korbar Date: Mon, 14 Mar 2022 10:25:27 +0100 Subject: [PATCH] Improve fix for CVE-2022-25236 Related: CVE-2022-25236 --- ...nst-malicious-namespace-declarations.patch | 205 +++++++++++++----- expat.spec | 6 +- 2 files changed, 159 insertions(+), 52 deletions(-) diff --git a/expat-2.2.10-Protect-against-malicious-namespace-declarations.patch b/expat-2.2.10-Protect-against-malicious-namespace-declarations.patch index 6dfb5ec..9f68836 100644 --- a/expat-2.2.10-Protect-against-malicious-namespace-declarations.patch +++ b/expat-2.2.10-Protect-against-malicious-namespace-declarations.patch @@ -1,17 +1,14 @@ -From 6881a4fc8596307ab9ff2e85e605afa2e413ab71 Mon Sep 17 00:00:00 2001 -From: Sebastian Pipping -Date: Sat, 12 Feb 2022 00:19:13 +0100 -Subject: [PATCH 1/4] lib: Fix (harmless) use of uninitialized memory +commit 5c47ae80738d0985babf06a023b3845169682064 +Author: Tomas Korbar +Date: Mon Mar 14 10:22:37 2022 +0100 ---- - expat/lib/xmlparse.c | 6 ++---- - 1 file changed, 2 insertions(+), 4 deletions(-) + Protect against malicious namespace declarations diff --git a/lib/xmlparse.c b/lib/xmlparse.c -index 902895d5..c768f856 100644 +index 5c3f573..901abbf 100644 --- a/lib/xmlparse.c +++ b/lib/xmlparse.c -@@ -718,8 +718,7 @@ XML_ParserCreate(const XML_Char *encodingName) { +@@ -638,8 +638,7 @@ XML_ParserCreate(const XML_Char *encodingName) { XML_Parser XMLCALL XML_ParserCreateNS(const XML_Char *encodingName, XML_Char nsSep) { @@ -21,7 +18,7 @@ index 902895d5..c768f856 100644 return XML_ParserCreate_MM(encodingName, NULL, tmp); } -@@ -1344,8 +1343,7 @@ XML_ExternalEntityParserCreate(XML_Parser oldParser, const XML_Char *context, +@@ -1253,8 +1252,7 @@ XML_ExternalEntityParserCreate(XML_Parser oldParser, const XML_Char *context, would be otherwise. */ if (parser->m_ns) { @@ -31,54 +28,159 @@ index 902895d5..c768f856 100644 parser = parserCreate(encodingName, &parser->m_mem, tmp, newDtd); } else { parser = parserCreate(encodingName, &parser->m_mem, NULL, newDtd); - -From a2fe525e660badd64b6c557c2b1ec26ddc07f6e4 Mon Sep 17 00:00:00 2001 -From: Sebastian Pipping -Date: Sat, 12 Feb 2022 01:09:29 +0100 -Subject: [PATCH 2/4] lib: Protect against malicious namespace declarations - (CVE-2022-25236) - ---- - expat/lib/xmlparse.c | 11 +++++++++++ - 1 file changed, 11 insertions(+) - -diff --git a/lib/xmlparse.c b/lib/xmlparse.c -index c768f856..a3aef88c 100644 ---- a/lib/xmlparse.c -+++ b/lib/xmlparse.c -@@ -3754,6 +3754,17 @@ addBinding(XML_Parser parser, PREFIX *prefix, const ATTRIBUTE_ID *attId, +@@ -3526,6 +3524,117 @@ storeAtts(XML_Parser parser, const ENCODING *enc, const char *attStr, + return XML_ERROR_NONE; + } + ++static XML_Bool ++is_rfc3986_uri_char(XML_Char candidate) { ++ // For the RFC 3986 ANBF grammar see ++ // https://datatracker.ietf.org/doc/html/rfc3986#appendix-A ++ ++ switch (candidate) { ++ // From rule "ALPHA" (uppercase half) ++ case 'A': ++ case 'B': ++ case 'C': ++ case 'D': ++ case 'E': ++ case 'F': ++ case 'G': ++ case 'H': ++ case 'I': ++ case 'J': ++ case 'K': ++ case 'L': ++ case 'M': ++ case 'N': ++ case 'O': ++ case 'P': ++ case 'Q': ++ case 'R': ++ case 'S': ++ case 'T': ++ case 'U': ++ case 'V': ++ case 'W': ++ case 'X': ++ case 'Y': ++ case 'Z': ++ ++ // From rule "ALPHA" (lowercase half) ++ case 'a': ++ case 'b': ++ case 'c': ++ case 'd': ++ case 'e': ++ case 'f': ++ case 'g': ++ case 'h': ++ case 'i': ++ case 'j': ++ case 'k': ++ case 'l': ++ case 'm': ++ case 'n': ++ case 'o': ++ case 'p': ++ case 'q': ++ case 'r': ++ case 's': ++ case 't': ++ case 'u': ++ case 'v': ++ case 'w': ++ case 'x': ++ case 'y': ++ case 'z': ++ ++ // From rule "DIGIT" ++ case '0': ++ case '1': ++ case '2': ++ case '3': ++ case '4': ++ case '5': ++ case '6': ++ case '7': ++ case '8': ++ case '9': ++ ++ // From rule "pct-encoded" ++ case '%': ++ ++ // From rule "unreserved" ++ case '-': ++ case '.': ++ case '_': ++ case '~': ++ ++ // From rule "gen-delims" ++ case ':': ++ case '/': ++ case '?': ++ case '#': ++ case '[': ++ case ']': ++ case '@': ++ ++ // From rule "sub-delims" ++ case '!': ++ case '$': ++ case '&': ++ case '\'': ++ case '(': ++ case ')': ++ case '*': ++ case '+': ++ case ',': ++ case ';': ++ case '=': ++ return XML_TRUE; ++ ++ default: ++ return XML_FALSE; ++ } ++} ++ + /* addBinding() overwrites the value of prefix->binding without checking. + Therefore one must keep track of the old value outside of addBinding(). + */ +@@ -3581,6 +3690,29 @@ addBinding(XML_Parser parser, PREFIX *prefix, const ATTRIBUTE_ID *attId, if (! mustBeXML && isXMLNS && (len > xmlnsLen || uri[len] != xmlnsNamespace[len])) isXMLNS = XML_FALSE; + -+ // NOTE: While Expat does not validate namespace URIs against RFC 3986, -+ // we have to at least make sure that the XML processor on top of -+ // Expat (that is splitting tag names by namespace separator into -+ // 2- or 3-tuples (uri-local or uri-local-prefix)) cannot be confused -+ // by an attacker putting additional namespace separator characters -+ // into namespace declarations. That would be ambiguous and not to -+ // be expected. -+ if (parser->m_ns && (uri[len] == parser->m_namespaceSeparator)) { ++ // NOTE: While Expat does not validate namespace URIs against RFC 3986 ++ // today (and is not REQUIRED to do so with regard to the XML 1.0 ++ // namespaces specification) we have to at least make sure, that ++ // the application on top of Expat (that is likely splitting expanded ++ // element names ("qualified names") of form ++ // "[uri sep] local [sep prefix] '\0'" back into 1, 2 or 3 pieces ++ // in its element handler code) cannot be confused by an attacker ++ // putting additional namespace separator characters into namespace ++ // declarations. That would be ambiguous and not to be expected. ++ // ++ // While the HTML API docs of function XML_ParserCreateNS have been ++ // advising against use of a namespace separator character that can ++ // appear in a URI for >20 years now, some widespread applications ++ // are using URI characters (':' (colon) in particular) for a ++ // namespace separator, in practice. To keep these applications ++ // functional, we only reject namespaces URIs containing the ++ // application-chosen namespace separator if the chosen separator ++ // is a non-URI character with regard to RFC 3986. ++ if (parser->m_ns && (uri[len] == parser->m_namespaceSeparator) ++ && ! is_rfc3986_uri_char(uri[len])) { + return XML_ERROR_SYNTAX; + } } isXML = isXML && len == xmlLen; isXMLNS = isXMLNS && len == xmlnsLen; - -From 2de077423fb22750ebea599677d523b53cb93b1d Mon Sep 17 00:00:00 2001 -From: Sebastian Pipping -Date: Sat, 12 Feb 2022 00:51:43 +0100 -Subject: [PATCH 3/4] tests: Cover CVE-2022-25236 - ---- - expat/tests/runtests.c | 30 ++++++++++++++++++++++++++++++ - 1 file changed, 30 insertions(+) - diff --git a/tests/runtests.c b/tests/runtests.c -index d07203f2..bc5344b1 100644 +index f03e008..40172d2 100644 --- a/tests/runtests.c +++ b/tests/runtests.c -@@ -7220,6 +7220,35 @@ START_TEST(test_ns_double_colon_doctype) { +@@ -7233,6 +7233,37 @@ START_TEST(test_ns_double_colon_doctype) { } END_TEST @@ -86,16 +188,18 @@ index d07203f2..bc5344b1 100644 + struct test_case { + enum XML_Status expectedStatus; + const char *doc; ++ XML_Char namesep; + }; + struct test_case cases[] = { -+ {XML_STATUS_OK, ""}, -+ {XML_STATUS_ERROR, ""}, ++ {XML_STATUS_OK, "", XCS('\n')}, ++ {XML_STATUS_ERROR, "", XCS('\n')}, ++ {XML_STATUS_OK, "", XCS(':')}, + }; + + size_t i = 0; + size_t failCount = 0; + for (; i < sizeof(cases) / sizeof(cases[0]); i++) { -+ XML_Parser parser = XML_ParserCreateNS(NULL, '\n'); ++ 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) @@ -114,7 +218,7 @@ index d07203f2..bc5344b1 100644 /* Control variable; the number of times duff_allocator() will successfully * allocate */ #define ALLOC_ALWAYS_SUCCEED (-1) -@@ -11905,6 +11934,7 @@ make_suite(void) { +@@ -11527,6 +11558,7 @@ make_suite(void) { tcase_add_test(tc_namespace, test_ns_utf16_doctype); tcase_add_test(tc_namespace, test_ns_invalid_doctype); tcase_add_test(tc_namespace, test_ns_double_colon_doctype); @@ -122,4 +226,3 @@ index d07203f2..bc5344b1 100644 suite_add_tcase(s, tc_misc); tcase_add_checked_fixture(tc_misc, NULL, basic_teardown); - diff --git a/expat.spec b/expat.spec index ca6088a..a516d10 100644 --- a/expat.spec +++ b/expat.spec @@ -3,7 +3,7 @@ Summary: An XML parser library Name: expat Version: %(echo %{unversion} | sed 's/_/./g') -Release: 10%{?dist} +Release: 11%{?dist} Source: https://github.com/libexpat/libexpat/archive/R_%{unversion}.tar.gz#/expat-%{version}.tar.gz URL: https://libexpat.github.io/ License: MIT @@ -89,6 +89,10 @@ make check %{_libdir}/lib*.a %changelog +* Mon Mar 14 2022 Tomas Korbar - 2.2.10-11 +- Improve fix for CVE-2022-25236 +- Related: CVE-2022-25236 + * Mon Feb 28 2022 Tomas Korbar - 2.2.10-10 - Fix multiple CVEs - CVE-2022-25236 expat: namespace-separator characters in "xmlns[:prefix]" attribute values can lead to arbitrary code execution