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
117
118
119
120
121
122
123
124
125
126
127
128
129
|
From 6881a4fc8596307ab9ff2e85e605afa2e413ab71 Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Sat, 12 Feb 2022 00:19:13 +0100
Subject: [PATCH] lib: Fix (harmless) use of uninitialized memory
Upstream-Status: Backport
https://github.com/libexpat/libexpat/pull/561/commits
CVE: CVE-2022-25236
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
expat/lib/xmlparse.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/xmlparse.c b/lib/xmlparse.c
index 902895d5..c768f856 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);
From a2fe525e660badd64b6c557c2b1ec26ddc07f6e4 Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Sat, 12 Feb 2022 01:09:29 +0100
Subject: [PATCH] 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,
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;
From 2de077423fb22750ebea599677d523b53cb93b1d Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Sat, 12 Feb 2022 00:51:43 +0100
Subject: [PATCH] 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
--- 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);
|
