1 |
Backport of: |
2 |
|
3 |
From 652dd12a858989b14eed4e84e453059cd3ba340e Mon Sep 17 00:00:00 2001 |
4 |
From: Nick Wellnhofer <wellnhofer@aevum.de> |
5 |
Date: Tue, 8 Feb 2022 03:29:24 +0100 |
6 |
Subject: [PATCH] [CVE-2022-23308] Use-after-free of ID and IDREF attributes |
7 |
|
8 |
If a document is parsed with XML_PARSE_DTDVALID and without |
9 |
XML_PARSE_NOENT, the value of ID attributes has to be normalized after |
10 |
potentially expanding entities in xmlRemoveID. Otherwise, later calls |
11 |
to xmlGetID can return a pointer to previously freed memory. |
12 |
|
13 |
ID attributes which are empty or contain only whitespace after |
14 |
entity expansion are affected in a similar way. This is fixed by |
15 |
not storing such attributes in the ID table. |
16 |
|
17 |
The test to detect streaming mode when validating against a DTD was |
18 |
broken. In connection with the defects above, this could result in a |
19 |
use-after-free when using the xmlReader interface with validation. |
20 |
Fix detection of streaming mode to avoid similar issues. (This changes |
21 |
the expected result of a test case. But as far as I can tell, using the |
22 |
XML reader with XIncludes referencing the root document never worked |
23 |
properly, anyway.) |
24 |
|
25 |
All of these issues can result in denial of service. Using xmlReader |
26 |
with validation could result in disclosure of memory via the error |
27 |
channel, typically stderr. The security impact of xmlGetID returning |
28 |
a pointer to freed memory depends on the application. The typical use |
29 |
case of calling xmlGetID on an unmodified document is not affected. |
30 |
--- |
31 |
result/XInclude/ns1.xml.rdr | 2 +- |
32 |
valid.c | 88 +++++++++++++++++++++++-------------- |
33 |
2 files changed, 56 insertions(+), 34 deletions(-) |
34 |
|
35 |
diff --git a/valid.c b/valid.c |
36 |
index 5ee391c0..8e596f1d 100644 |
37 |
--- a/valid.c |
38 |
+++ b/valid.c |
39 |
@@ -479,6 +479,35 @@ nodeVPop(xmlValidCtxtPtr ctxt) |
40 |
return (ret); |
41 |
} |
42 |
|
43 |
+/** |
44 |
+ * xmlValidNormalizeString: |
45 |
+ * @str: a string |
46 |
+ * |
47 |
+ * Normalize a string in-place. |
48 |
+ */ |
49 |
+static void |
50 |
+xmlValidNormalizeString(xmlChar *str) { |
51 |
+ xmlChar *dst; |
52 |
+ const xmlChar *src; |
53 |
+ |
54 |
+ if (str == NULL) |
55 |
+ return; |
56 |
+ src = str; |
57 |
+ dst = str; |
58 |
+ |
59 |
+ while (*src == 0x20) src++; |
60 |
+ while (*src != 0) { |
61 |
+ if (*src == 0x20) { |
62 |
+ while (*src == 0x20) src++; |
63 |
+ if (*src != 0) |
64 |
+ *dst++ = 0x20; |
65 |
+ } else { |
66 |
+ *dst++ = *src++; |
67 |
+ } |
68 |
+ } |
69 |
+ *dst = 0; |
70 |
+} |
71 |
+ |
72 |
#ifdef DEBUG_VALID_ALGO |
73 |
static void |
74 |
xmlValidPrintNode(xmlNodePtr cur) { |
75 |
@@ -2607,6 +2636,24 @@ xmlDumpNotationTable(xmlBufferPtr buf, xmlNotationTablePtr table) { |
76 |
(xmlDictOwns(dict, (const xmlChar *)(str)) == 0))) \ |
77 |
xmlFree((char *)(str)); |
78 |
|
79 |
+static int |
80 |
+xmlIsStreaming(xmlValidCtxtPtr ctxt) { |
81 |
+ xmlParserCtxtPtr pctxt; |
82 |
+ |
83 |
+ if (ctxt == NULL) |
84 |
+ return(0); |
85 |
+ /* |
86 |
+ * These magic values are also abused to detect whether we're validating |
87 |
+ * while parsing a document. In this case, userData points to the parser |
88 |
+ * context. |
89 |
+ */ |
90 |
+ if ((ctxt->finishDtd != XML_CTXT_FINISH_DTD_0) && |
91 |
+ (ctxt->finishDtd != XML_CTXT_FINISH_DTD_1)) |
92 |
+ return(0); |
93 |
+ pctxt = ctxt->userData; |
94 |
+ return(pctxt->parseMode == XML_PARSE_READER); |
95 |
+} |
96 |
+ |
97 |
/** |
98 |
* xmlFreeID: |
99 |
* @not: A id |
100 |
@@ -2650,7 +2697,7 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value, |
101 |
if (doc == NULL) { |
102 |
return(NULL); |
103 |
} |
104 |
- if (value == NULL) { |
105 |
+ if ((value == NULL) || (value[0] == 0)) { |
106 |
return(NULL); |
107 |
} |
108 |
if (attr == NULL) { |
109 |
@@ -2681,7 +2728,7 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value, |
110 |
*/ |
111 |
ret->value = xmlStrdup(value); |
112 |
ret->doc = doc; |
113 |
- if ((ctxt != NULL) && (ctxt->vstateNr != 0)) { |
114 |
+ if (xmlIsStreaming(ctxt)) { |
115 |
/* |
116 |
* Operating in streaming mode, attr is gonna disappear |
117 |
*/ |
118 |
@@ -2820,6 +2867,7 @@ xmlRemoveID(xmlDocPtr doc, xmlAttrPtr attr) { |
119 |
ID = xmlNodeListGetString(doc, attr->children, 1); |
120 |
if (ID == NULL) |
121 |
return(-1); |
122 |
+ xmlValidNormalizeString(ID); |
123 |
|
124 |
id = xmlHashLookup(table, ID); |
125 |
if (id == NULL || id->attr != attr) { |
126 |
@@ -3009,7 +3057,7 @@ xmlAddRef(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value, |
127 |
* fill the structure. |
128 |
*/ |
129 |
ret->value = xmlStrdup(value); |
130 |
- if ((ctxt != NULL) && (ctxt->vstateNr != 0)) { |
131 |
+ if (xmlIsStreaming(ctxt)) { |
132 |
/* |
133 |
* Operating in streaming mode, attr is gonna disappear |
134 |
*/ |
135 |
@@ -4028,8 +4076,7 @@ xmlValidateAttributeValue2(xmlValidCtxtPtr ctxt, xmlDocPtr doc, |
136 |
xmlChar * |
137 |
xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc, |
138 |
xmlNodePtr elem, const xmlChar *name, const xmlChar *value) { |
139 |
- xmlChar *ret, *dst; |
140 |
- const xmlChar *src; |
141 |
+ xmlChar *ret; |
142 |
xmlAttributePtr attrDecl = NULL; |
143 |
int extsubset = 0; |
144 |
|
145 |
@@ -4070,19 +4117,7 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc, |
146 |
ret = xmlStrdup(value); |
147 |
if (ret == NULL) |
148 |
return(NULL); |
149 |
- src = value; |
150 |
- dst = ret; |
151 |
- while (*src == 0x20) src++; |
152 |
- while (*src != 0) { |
153 |
- if (*src == 0x20) { |
154 |
- while (*src == 0x20) src++; |
155 |
- if (*src != 0) |
156 |
- *dst++ = 0x20; |
157 |
- } else { |
158 |
- *dst++ = *src++; |
159 |
- } |
160 |
- } |
161 |
- *dst = 0; |
162 |
+ xmlValidNormalizeString(ret); |
163 |
if ((doc->standalone) && (extsubset == 1) && (!xmlStrEqual(value, ret))) { |
164 |
xmlErrValidNode(ctxt, elem, XML_DTD_NOT_STANDALONE, |
165 |
"standalone: %s on %s value had to be normalized based on external subset declaration\n", |
166 |
@@ -4114,8 +4149,7 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc, |
167 |
xmlChar * |
168 |
xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem, |
169 |
const xmlChar *name, const xmlChar *value) { |
170 |
- xmlChar *ret, *dst; |
171 |
- const xmlChar *src; |
172 |
+ xmlChar *ret; |
173 |
xmlAttributePtr attrDecl = NULL; |
174 |
|
175 |
if (doc == NULL) return(NULL); |
176 |
@@ -4145,19 +4179,7 @@ xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem, |
177 |
ret = xmlStrdup(value); |
178 |
if (ret == NULL) |
179 |
return(NULL); |
180 |
- src = value; |
181 |
- dst = ret; |
182 |
- while (*src == 0x20) src++; |
183 |
- while (*src != 0) { |
184 |
- if (*src == 0x20) { |
185 |
- while (*src == 0x20) src++; |
186 |
- if (*src != 0) |
187 |
- *dst++ = 0x20; |
188 |
- } else { |
189 |
- *dst++ = *src++; |
190 |
- } |
191 |
- } |
192 |
- *dst = 0; |
193 |
+ xmlValidNormalizeString(ret); |
194 |
return(ret); |
195 |
} |
196 |
|
197 |
-- |
198 |
GitLab |
199 |
|