1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
|
Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/2cc97e87]
CVE: CVE-2022-25236
The commit is a merge commit, and this patch is created by:
$ git diff -p --stat 2cc97e87~ 2cc97e87
Remove modification for expat/Changes which fails to be applied.
Signed-off-by: Kai Kang <kai.kang@windriver.com>
commit 2cc97e875ef84da4bcf55156c83599116f7523b4 (from d477fdd284468f2ab822024e75702f2c1b254f42)
Merge: d477fdd2 e4d7e497
Author: Sebastian Pipping <sebastian@pipping.org>
Date: Fri Feb 18 18:01:27 2022 +0100
Merge pull request #561 from libexpat/namesep-security
[CVE-2022-25236] lib: Protect against insertion of namesep characters into namespace URIs
---
expat/Changes | 16 ++++++++++++++++
expat/lib/xmlparse.c | 17 +++++++++++++----
expat/tests/runtests.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/lib/xmlparse.c b/lib/xmlparse.c
index 7376aab1..c98e2e9f 100644
--- a/lib/xmlparse.c
+++ b/lib/xmlparse.c
@@ -718,8 +718,7 @@ XML_ParserCreate(const XML_Char *encodingName) {
XML_Parser XMLCALL
XML_ParserCreateNS(const XML_Char *encodingName, XML_Char nsSep) {
- XML_Char tmp[2];
- *tmp = nsSep;
+ XML_Char tmp[2] = {nsSep, 0};
return XML_ParserCreate_MM(encodingName, NULL, tmp);
}
@@ -1344,8 +1343,7 @@ XML_ExternalEntityParserCreate(XML_Parser oldParser, const XML_Char *context,
would be otherwise.
*/
if (parser->m_ns) {
- XML_Char tmp[2];
- *tmp = parser->m_namespaceSeparator;
+ XML_Char tmp[2] = {parser->m_namespaceSeparator, 0};
parser = parserCreate(encodingName, &parser->m_mem, tmp, newDtd);
} else {
parser = parserCreate(encodingName, &parser->m_mem, NULL, newDtd);
@@ -3761,6 +3759,17 @@ 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)) {
+ return XML_ERROR_SYNTAX;
+ }
}
isXML = isXML && len == xmlLen;
isXMLNS = isXMLNS && len == xmlnsLen;
diff --git a/tests/runtests.c b/tests/runtests.c
index d07203f2..bc5344b1 100644
--- a/tests/runtests.c
+++ b/tests/runtests.c
@@ -7220,6 +7220,35 @@ START_TEST(test_ns_double_colon_doctype) {
}
END_TEST
+START_TEST(test_ns_separator_in_uri) {
+ struct test_case {
+ enum XML_Status expectedStatus;
+ const char *doc;
+ };
+ struct test_case cases[] = {
+ {XML_STATUS_OK, "<doc xmlns='one_two' />"},
+ {XML_STATUS_ERROR, "<doc xmlns='one
two' />"},
+ };
+
+ size_t i = 0;
+ size_t failCount = 0;
+ for (; i < sizeof(cases) / sizeof(cases[0]); i++) {
+ XML_Parser parser = XML_ParserCreateNS(NULL, '\n');
+ XML_SetElementHandler(parser, dummy_start_element, dummy_end_element);
+ if (XML_Parse(parser, cases[i].doc, (int)strlen(cases[i].doc),
+ /*isFinal*/ XML_TRUE)
+ != cases[i].expectedStatus) {
+ failCount++;
+ }
+ XML_ParserFree(parser);
+ }
+
+ if (failCount) {
+ fail("Namespace separator handling is broken");
+ }
+}
+END_TEST
+
/* Control variable; the number of times duff_allocator() will successfully
* allocate */
#define ALLOC_ALWAYS_SUCCEED (-1)
@@ -11905,6 +11934,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);
+ tcase_add_test(tc_namespace, test_ns_separator_in_uri);
suite_add_tcase(s, tc_misc);
tcase_add_checked_fixture(tc_misc, NULL, basic_teardown);
|
