CVE-2022-23308: Use After Free (CWE-416)
value: valid.c in libxml2 before 2.9.13 has a use-after-free of ID and IDREF attributes.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7.5
Published: 2022-02-26T05:15Z
Repository: https://github.com/GNOME/libxml2
Language: C
Files changed: result/XInclude/ns1.xml.rdr, valid.c
Commit date: 2022-02-19 19:26:42 +0100
Commit message: [CVE-2022-23308] Use-after-free of ID and IDREF attributes

If a document is parsed with XML_PARSE_DTDVALID and without
XML_PARSE_NOENT, the value of ID attributes has to be normalized after
potentially expanding entities in xmlRemoveID. Otherwise, later calls
to xmlGetID can return a pointer to previously freed memory.

ID attributes which are empty or contain only whitespace after
entity expansion are affected in a similar way. This is fixed by
not storing such attributes in the ID table.

The test to detect streaming mode when validating against a DTD was
broken. In connection with the defects above, this could result in a
use-after-free when using the xmlReader interface with validation.
Fix detection of streaming mode to avoid similar issues. (This changes
the expected result of a test case. But as far as I can tell, using the
XML reader with XIncludes referencing the root document never worked
properly, anyway.)

All of these issues can result in denial of service. Using xmlReader
with validation could result in disclosure of memory via the error
channel, typically stderr. The security impact of xmlGetID returning
a pointer to freed memory depends on the application. The typical use
case of calling xmlGetID on an unmodified document is not affected.

Diff: diff --git a/result/XInclude/ns1.xml.rdr b/result/XInclude/ns1.xml.rdr
index f23702f57..9a3a5e76a 100644
--- a/result/XInclude/ns1.xml.rdr
+++ b/result/XInclude/ns1.xml.rdr
@@ -1,9 +1,9 @@
 0 1 doc 0 0
 1 14 #text 0 1 
     
-1 1 ns:elem 1 0
+1 1 xi:include 1 0
 1 14 #text 0 1 
     
 1 1 elem 0 0
 2 14 #text 0 1 
         
diff --git a/valid.c b/valid.c
index 5ee391c04..8e596f1db 100644
--- a/valid.c
+++ b/valid.c
@@ -477,10 +477,39 @@ nodeVPop(xmlValidCtxtPtr ctxt)
     ret = ctxt->nodeTab[ctxt->nodeNr];
     ctxt->nodeTab[ctxt->nodeNr] = NULL;
     return (ret);
 }
 
+/**
+ * xmlValidNormalizeString:
+ * @str: a string
+ *
+ * Normalize a string in-place.
+ */
+static void
+xmlValidNormalizeString(xmlChar *str) {
+    xmlChar *dst;
+    const xmlChar *src;
+
+    if (str == NULL)
+        return;
+    src = str;
+    dst = str;
+
+    while (*src == 0x20) src++;
+    while (*src != 0) {
+	if (*src == 0x20) {
+	    while (*src == 0x20) src++;
+	    if (*src != 0)
+		*dst++ = 0x20;
+	} else {
+	    *dst++ = *src++;
+	}
+    }
+    *dst = 0;
+}
+
 #ifdef DEBUG_VALID_ALGO
 static void
 xmlValidPrintNode(xmlNodePtr cur) {
     if (cur == NULL) {
 	xmlGenericError(xmlGenericErrorContext, "null");
@@ -2605,10 +2634,28 @@ xmlDumpNotationTable(xmlBufferPtr buf, xmlNotationTablePtr table) {
 #define DICT_FREE(str)						\
 	if ((str) && ((!dict) ||				\
 	    (xmlDictOwns(dict, (const xmlChar *)(str)) == 0)))	\
 	    xmlFree((char *)(str));
 
+static int
+xmlIsStreaming(xmlValidCtxtPtr ctxt) {
+    xmlParserCtxtPtr pctxt;
+
+    if (ctxt == NULL)
+        return(0);
+    /*
+     * These magic values are also abused to detect whether we're validating
+     * while parsing a document. In this case, userData points to the parser
+     * context.
+     */
+    if ((ctxt->finishDtd != XML_CTXT_FINISH_DTD_0) &&
+        (ctxt->finishDtd != XML_CTXT_FINISH_DTD_1))
+        return(0);
+    pctxt = ctxt->userData;
+    return(pctxt->parseMode == XML_PARSE_READER);
+}
+
 /**
  * xmlFreeID:
  * @not:  A id
  *
  * Deallocate the memory used by an id definition
@@ -2648,11 +2695,11 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
     xmlIDTablePtr table;
 
     if (doc == NULL) {
 	return(NULL);
     }
-    if (value == NULL) {
+    if ((value == NULL) || (value[0] == 0)) {
 	return(NULL);
     }
     if (attr == NULL) {
 	return(NULL);
     }
@@ -2679,11 +2726,11 @@ xmlAddID(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
     /*
      * fill the structure.
      */
     ret->value = xmlStrdup(value);
     ret->doc = doc;
-    if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
+    if (xmlIsStreaming(ctxt)) {
 	/*
 	 * Operating in streaming mode, attr is gonna disappear
 	 */
 	if (doc->dict != NULL)
 	    ret->name = xmlDictLookup(doc->dict, attr->name, -1);
@@ -2818,10 +2865,11 @@ xmlRemoveID(xmlDocPtr doc, xmlAttrPtr attr) {
         return(-1);
 
     ID = xmlNodeListGetString(doc, attr->children, 1);
     if (ID == NULL)
         return(-1);
+    xmlValidNormalizeString(ID);
 
     id = xmlHashLookup(table, ID);
     if (id == NULL || id->attr != attr) {
         xmlFree(ID);
         return(-1);
@@ -3007,11 +3055,11 @@ xmlAddRef(xmlValidCtxtPtr ctxt, xmlDocPtr doc, const xmlChar *value,
 
     /*
      * fill the structure.
      */
     ret->value = xmlStrdup(value);
-    if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
+    if (xmlIsStreaming(ctxt)) {
 	/*
 	 * Operating in streaming mode, attr is gonna disappear
 	 */
 	ret->name = xmlStrdup(attr->name);
 	ret->attr = NULL;
@@ -4026,12 +4074,11 @@ xmlValidateAttributeValue2(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
  */
 
 xmlChar *
 xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
 	     xmlNodePtr elem, const xmlChar *name, const xmlChar *value) {
-    xmlChar *ret, *dst;
-    const xmlChar *src;
+    xmlChar *ret;
     xmlAttributePtr attrDecl = NULL;
     int extsubset = 0;
 
     if (doc == NULL) return(NULL);
     if (elem == NULL) return(NULL);
@@ -4068,23 +4115,11 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
 	return(NULL);
 
     ret = xmlStrdup(value);
     if (ret == NULL)
 	return(NULL);
-    src = value;
-    dst = ret;
-    while (*src == 0x20) src++;
-    while (*src != 0) {
-	if (*src == 0x20) {
-	    while (*src == 0x20) src++;
-	    if (*src != 0)
-		*dst++ = 0x20;
-	} else {
-	    *dst++ = *src++;
-	}
-    }
-    *dst = 0;
+    xmlValidNormalizeString(ret);
     if ((doc->standalone) && (extsubset == 1) && (!xmlStrEqual(value, ret))) {
 	xmlErrValidNode(ctxt, elem, XML_DTD_NOT_STANDALONE,
 "standalone: %s on %s value had to be normalized based on external subset declaration\n",
 	       name, elem->name, NULL);
 	ctxt->valid = 0;
@@ -4112,12 +4147,11 @@ xmlValidCtxtNormalizeAttributeValue(xmlValidCtxtPtr ctxt, xmlDocPtr doc,
  */
 
 xmlChar *
 xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem,
 			        const xmlChar *name, const xmlChar *value) {
-    xmlChar *ret, *dst;
-    const xmlChar *src;
+    xmlChar *ret;
     xmlAttributePtr attrDecl = NULL;
 
     if (doc == NULL) return(NULL);
     if (elem == NULL) return(NULL);
     if (name == NULL) return(NULL);
@@ -4143,23 +4177,11 @@ xmlValidNormalizeAttributeValue(xmlDocPtr doc, xmlNodePtr elem,
 	return(NULL);
 
     ret = xmlStrdup(value);
     if (ret == NULL)
 	return(NULL);
-    src = value;
-    dst = ret;
-    while (*src == 0x20) src++;
-    while (*src != 0) {
-	if (*src == 0x20) {
-	    while (*src == 0x20) src++;
-	    if (*src != 0)
-		*dst++ = 0x20;
-	} else {
-	    *dst++ = *src++;
-	}
-    }
-    *dst = 0;
+    xmlValidNormalizeString(ret);
     return(ret);
 }
 
 static void
 xmlValidateAttributeIdCallback(void *payload, void *data,

Vulnerable code: 1 1 ns:elem 1 0
    if (value == NULL) {
    if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
    if ((ctxt != NULL) && (ctxt->vstateNr != 0)) {
    xmlChar *ret, *dst;
    const xmlChar *src;
    src = value;
    dst = ret;
    while (*src == 0x20) src++;
    while (*src != 0) {
	if (*src == 0x20) {
	    while (*src == 0x20) src++;
	    if (*src != 0)
		*dst++ = 0x20;
	} else {
	    *dst++ = *src++;
	}
    }
    *dst = 0;
    xmlChar *ret, *dst;
    const xmlChar *src;
    src = value;
    dst = ret;
    while (*src == 0x20) src++;
    while (*src != 0) {
	if (*src == 0x20) {
	    while (*src == 0x20) src++;
	    if (*src != 0)
		*dst++ = 0x20;
	} else {
	    *dst++ = *src++;
	}
    }
    *dst = 0;
Fixed code: 1 1 xi:include 1 0
/**
 * xmlValidNormalizeString:
 * @str: a string
 *
 * Normalize a string in-place.
 */
static void
xmlValidNormalizeString(xmlChar *str) {
    xmlChar *dst;
    const xmlChar *src;

    if (str == NULL)
        return;
    src = str;
    dst = str;

    while (*src == 0x20) src++;
    while (*src != 0) {
	if (*src == 0x20) {
	    while (*src == 0x20) src++;
	    if (*src != 0)
		*dst++ = 0x20;
	} else {
	    *dst++ = *src++;
	}
    }
    *dst = 0;
}

static int
xmlIsStreaming(xmlValidCtxtPtr ctxt) {
    xmlParserCtxtPtr pctxt;

    if (ctxt == NULL)
        return(0);
    /*
     * These magic values are also abused to detect whether we're validating
     * while parsing a document. In this case, userData points to the parser
     * context.
     */
    if ((ctxt->finishDtd != XML_CTXT_FINISH_DTD_0) &&
        (ctxt->finishDtd != XML_CTXT_FINISH_DTD_1))
        return(0);
    pctxt = ctxt->userData;
    return(pctxt->parseMode == XML_PARSE_READER);
}

    if ((value == NULL) || (value[0] == 0)) {
    if (xmlIsStreaming(ctxt)) {
    xmlValidNormalizeString(ID);
    if (xmlIsStreaming(ctxt)) {
    xmlChar *ret;
    xmlValidNormalizeString(ret);
    xmlChar *ret;
    xmlValidNormalizeString(ret);