|
|
a7591b |
From 7f70302bfa9faeac9c9f7be8adf96d32c16acb72 Mon Sep 17 00:00:00 2001
|
|
|
a7591b |
From: Nick Wellnhofer <wellnhofer@aevum.de>
|
|
|
a7591b |
Date: Tue, 8 Feb 2022 03:29:24 +0100
|
|
|
a7591b |
Subject: [PATCH] [CVE-2022-23308] Use-after-free of ID and IDREF attributes
|
|
|
a7591b |
|
|
|
a7591b |
If a document is parsed with XML_PARSE_DTDVALID and without
|
|
|
a7591b |
XML_PARSE_NOENT, the value of ID attributes has to be normalized after
|
|
|
a7591b |
potentially expanding entities in xmlRemoveID. Otherwise, later calls
|
|
|
a7591b |
to xmlGetID can return a pointer to previously freed memory.
|
|
|
a7591b |
|
|
|
a7591b |
ID attributes which are empty or contain only whitespace after
|
|
|
a7591b |
entity expansion are affected in a similar way. This is fixed by
|
|
|
a7591b |
not storing such attributes in the ID table.
|
|
|
a7591b |
|
|
|
a7591b |
The test to detect streaming mode when validating against a DTD was
|
|
|
a7591b |
broken. In connection with the defects above, this could result in a
|
|
|
a7591b |
use-after-free when using the xmlReader interface with validation.
|
|
|
a7591b |
Fix detection of streaming mode to avoid similar issues. (This changes
|
|
|
a7591b |
the expected result of a test case. But as far as I can tell, using the
|
|
|
a7591b |
XML reader with XIncludes referencing the root document never worked
|
|
|
a7591b |
properly, anyway.)
|
|
|
a7591b |
|
|
|
a7591b |
All of these issues can result in denial of service. Using xmlReader
|
|
|
a7591b |
with validation could result in disclosure of memory via the error
|
|
|
a7591b |
channel, typically stderr. The security impact of xmlGetID returning
|
|
|
a7591b |
a pointer to freed memory depends on the application. The typical use
|
|
|
a7591b |
case of calling xmlGetID on an unmodified document is not affected.
|
|
|
a7591b |
---
|
|
|
a7591b |
valid.c | 88 +++++++++++++++++++++++++++++++++++----------------------
|
|
|
a7591b |
1 file changed, 55 insertions(+), 33 deletions(-)
|
|
|
a7591b |
|
|
|
a7591b |
diff --git a/valid.c b/valid.c
|
|
|
a7591b |
index a64b96be..5b81059f 100644
|
|
|
a7591b |
--- a/valid.c
|
|
|
a7591b |
+++ b/valid.c
|
|
|
a7591b |
@@ -479,6 +479,35 @@ nodeVPop(xmlValidCtxtPtr ctxt)
|
|
|
a7591b |
return (ret);
|
|
|
a7591b |
}
|
|
|
a7591b |
|
|
|
a7591b |
+/**
|
|
|
a7591b |
+ * xmlValidNormalizeString:
|
|
|
a7591b |
+ * @str: a string
|
|
|
a7591b |
+ *
|
|
|
a7591b |
+ * Normalize a string in-place.
|
|
|
a7591b |
+ */
|
|
|
a7591b |
+static void
|
|
|
a7591b |
+xmlValidNormalizeString(xmlChar *str) {
|
|
|
a7591b |
+ xmlChar *dst;
|
|
|
a7591b |
+ const xmlChar *src;
|
|
|
a7591b |
+
|
|
|
a7591b |
+ if (str == NULL)
|
|
|
a7591b |
+ return;
|
|
|
a7591b |
+ src = str;
|
|
|
a7591b |
+ dst = str;
|
|
|
a7591b |
+
|
|
|
a7591b |
+ while (*src == 0x20) src++;
|
|
|
a7591b |
+ while (*src != 0) {
|
|
|
a7591b |
+ if (*src == 0x20) {
|
|
|
a7591b |
+ while (*src == 0x20) src++;
|
|
|
a7591b |
+ if (*src != 0)
|
|
|
a7591b |
+ *dst++ = 0x20;
|
|
|
a7591b |
+ } else {
|
|
|
a7591b |
+ *dst++ = *src++;
|
|
|
a7591b |
+ }
|
|
|
a7591b |
+ }
|
|
|
a7591b |
+ *dst = 0;
|
|
|
a7591b |
+}
|
|
|
a7591b |
+
|
|
|
a7591b |
#ifdef DEBUG_VALID_ALGO
|
|
|
a7591b |
static void
|
|
|
a7591b |
xmlValidPrintNode(xmlNodePtr cur) {
|
|
|
a7591b |
@@ -2546,6 +2575,24 @@ xmlDumpNotationTable(xmlBufferPtr buf, xmlNotationTablePtr table) {
|
|
|
a7591b |
(xmlDictOwns(dict, (const xmlChar *)(str)) == 0))) \
|
|
|
a7591b |
xmlFree((char *)(str));
|
|
|
a7591b |
|
|
|
a7591b |
+static int
|
|
|
a7591b |
+xmlIsStreaming(xmlValidCtxtPtr ctxt) {
|
|
|
a7591b |
+ xmlParserCtxtPtr pctxt;
|
|
|
a7591b |
+
|
|
|
a7591b |
+ if (ctxt == NULL)
|
|
|
a7591b |
+ return(0);
|
|
|
a7591b |
+ /*
|
|
|
a7591b |
+ * These magic values are also abused to detect whether we're validating
|
|
|
a7591b |
+ * while parsing a document. In this case, userData points to the parser
|
|
|
a7591b |
+ * context.
|
|
|
a7591b |
+ */
|
|
|
a7591b |
+ if ((ctxt->finishDtd != XML_CTXT_FINISH_DTD_0) &&
|
|
|
a7591b |
+ (ctxt->finishDtd != XML_CTXT_FINISH_DTD_1))
|
|
|
a7591b |
+ return(0);
|
|
|
a7591b |
+ pctxt = ctxt->userData;
|
|
|
a7591b |
+ return(pctxt->parseMode == XML_PARSE_READER);
|
|
|
a7591b |
+}
|
|
|
a7591b |
+
|
|
|
a7591b |
/**
|
|
|
a7591b |
* xmlFreeID:
|
|
|
a7591b |
* @not: A id
|
|
|
a7591b |
@@ -2589,7 +2636,7 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
|
|
|
a7591b |
if (doc == NULL) {
|
|
|
a7591b |
return(NULL);
|
|
|
a7591b |
}
|
|
|
a7591b |
- if (value == NULL) {
|
|
|
a7591b |
+ if ((value == NULL) || (value[0] == 0)) {
|
|
|
a7591b |
return(NULL);
|
|
|
a7591b |
}
|
|
|
a7591b |
if (attr == NULL) {
|
|
|
a7591b |
@@ -2620,7 +2667,7 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
|
|
|
a7591b |
*/
|
|
|
a7591b |
ret->value = xmlStrdup(value);
|
|
|
a7591b |
ret->doc = doc;
|
|
|
a7591b |
- if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
|
|
|
a7591b |
+ if (xmlIsStreaming(ctxt)) {
|
|
|
a7591b |
/*
|
|
|
a7591b |
* Operating in streaming mode, attr is gonna disapear
|
|
|
a7591b |
*/
|
|
|
a7591b |
@@ -2754,6 +2801,7 @@ xmlRemoveID(xmlDocPtr doc, xmlAttrPtr attr) {
|
|
|
a7591b |
ID = xmlNodeListGetString(doc, attr->children, 1);
|
|
|
a7591b |
if (ID == NULL)
|
|
|
a7591b |
return(-1);
|
|
|
a7591b |
+ xmlValidNormalizeString(ID);
|
|
|
a7591b |
|
|
|
a7591b |
id = xmlHashLookup(table, ID);
|
|
|
a7591b |
if (id == NULL || id->attr != attr) {
|
|
|
a7591b |
@@ -2942,7 +2990,7 @@ xmlAddRef(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
|
|
|
a7591b |
* fill the structure.
|
|
|
a7591b |
*/
|
|
|
a7591b |
ret->value = xmlStrdup(value);
|
|
|
a7591b |
- if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
|
|
|
a7591b |
+ if (xmlIsStreaming(ctxt)) {
|
|
|
a7591b |
/*
|
|
|
a7591b |
* Operating in streaming mode, attr is gonna disapear
|
|
|
a7591b |
*/
|
|
|
a7591b |
@@ -3962,8 +4010,7 @@ xmlValidateAttributeValue2(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
|
|
|
a7591b |
xmlChar *
|
|
|
a7591b |
xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
|
|
|
a7591b |
xmlNodePtr elem, const xmlChar *name, const xmlChar *value) {
|
|
|
a7591b |
- xmlChar *ret, *dst;
|
|
|
a7591b |
- const xmlChar *src;
|
|
|
a7591b |
+ xmlChar *ret;
|
|
|
a7591b |
xmlAttributePtr attrDecl = NULL;
|
|
|
a7591b |
int extsubset = 0;
|
|
|
a7591b |
|
|
|
a7591b |
@@ -4004,19 +4051,7 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
|
|
|
a7591b |
ret = xmlStrdup(value);
|
|
|
a7591b |
if (ret == NULL)
|
|
|
a7591b |
return(NULL);
|
|
|
a7591b |
- src = value;
|
|
|
a7591b |
- dst = ret;
|
|
|
a7591b |
- while (*src == 0x20) src++;
|
|
|
a7591b |
- while (*src != 0) {
|
|
|
a7591b |
- if (*src == 0x20) {
|
|
|
a7591b |
- while (*src == 0x20) src++;
|
|
|
a7591b |
- if (*src != 0)
|
|
|
a7591b |
- *dst++ = 0x20;
|
|
|
a7591b |
- } else {
|
|
|
a7591b |
- *dst++ = *src++;
|
|
|
a7591b |
- }
|
|
|
a7591b |
- }
|
|
|
a7591b |
- *dst = 0;
|
|
|
a7591b |
+ xmlValidNormalizeString(ret);
|
|
|
a7591b |
if ((doc->standalone) && (extsubset == 1) && (!xmlStrEqual(value, ret))) {
|
|
|
a7591b |
xmlErrValidNode(ctxt, elem, XML_DTD_NOT_STANDALONE,
|
|
|
a7591b |
"standalone: %s on %s value had to be normalized based on external subset declaration\n",
|
|
|
a7591b |
@@ -4048,8 +4083,7 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
|
|
|
a7591b |
xmlChar *
|
|
|
a7591b |
xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem,
|
|
|
a7591b |
const xmlChar *name, const xmlChar *value) {
|
|
|
a7591b |
- xmlChar *ret, *dst;
|
|
|
a7591b |
- const xmlChar *src;
|
|
|
a7591b |
+ xmlChar *ret;
|
|
|
a7591b |
xmlAttributePtr attrDecl = NULL;
|
|
|
a7591b |
|
|
|
a7591b |
if (doc == NULL) return(NULL);
|
|
|
a7591b |
@@ -4079,19 +4113,7 @@ xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem,
|
|
|
a7591b |
ret = xmlStrdup(value);
|
|
|
a7591b |
if (ret == NULL)
|
|
|
a7591b |
return(NULL);
|
|
|
a7591b |
- src = value;
|
|
|
a7591b |
- dst = ret;
|
|
|
a7591b |
- while (*src == 0x20) src++;
|
|
|
a7591b |
- while (*src != 0) {
|
|
|
a7591b |
- if (*src == 0x20) {
|
|
|
a7591b |
- while (*src == 0x20) src++;
|
|
|
a7591b |
- if (*src != 0)
|
|
|
a7591b |
- *dst++ = 0x20;
|
|
|
a7591b |
- } else {
|
|
|
a7591b |
- *dst++ = *src++;
|
|
|
a7591b |
- }
|
|
|
a7591b |
- }
|
|
|
a7591b |
- *dst = 0;
|
|
|
a7591b |
+ xmlValidNormalizeString(ret);
|
|
|
a7591b |
return(ret);
|
|
|
a7591b |
}
|
|
|
a7591b |
|
|
|
a7591b |
--
|
|
|
a7591b |
2.35.1
|
|
|
a7591b |
|