[libxml2/fix-ownership-of-xmlAttrPtr-name-in-xmlSetTreeDoc] Fix use-after-free bugs when calling xmlTextReaderClose() before xmlFreeTextReader() on post-validat

[David Kilzer <ddkilzer src gnome org>]

commit bc977ef53eee21273405ddffd1a220629b73bc1b
Author: David Kilzer <ddkilzer apple com>
Date:   Sun Apr 10 20:02:47 2022 -0700

    Fix use-after-free bugs when calling xmlTextReaderClose() before xmlFreeTextReader() on post-validating 
parser
    
    When creating an xmlTextReaderPtr using xmlReaderForMemory(),
    there are two optional API functions that can be used:
    - xmlTextReaderClose() may be called prior to calling
      xmlFreeTextReader() to free parsing resources and close the
      xmlTextReaderPtr without freeing it.
    - xmlTextReaderCurrentDoc() may be called to return an
      xmlDocPtr that's owned by the caller, and must be free using
      xmlFreeDoc() after calling xmlFreeTextReader().
    
    The use-after-free issues occur when calling
    xmlTextReaderClose() before xmlFreeTextReader(), with different
    issues occurring depending on whether xmlTextReaderCurrentDoc()
    is also called.
    
    * fuzz/xml.c:
    (LLVMFuzzerTestOneInput):
    - Add support for testing xmlTextReaderCurrentDoc()/xmlFreeDoc()
      and xmlTextReaderClose() with xmlFreeTextReader().  This
      change finds the remaining bug from 2009 almost immediately.
    
    * xmlreader.c:
    (xmlFreeTextReader):
    - Move code to xmlTextReaderClose(), remove duplicate code, and
      call xmlTextReaderClose() if it hasn't been called yet.
    (xmlTextReaderClose):
    - Move call to xmlFreeNode(reader->faketext) from
      xmlFreeTextReader() to fix a use-after-free bug when calling
      xmlTextReaderClose() before xmlFreeTextReader(), but not when
      using xmlTextReaderCurrentDoc().  The bug was introduced in
      2002 by commit beb70bd39.  In 2009 commit f4653dcd8 fixed the
      use-after-free that occurred every time xmlFreeTextReader()
      was called, but not the case where xmlTextReaderClose() was
      called first.
    - Move post-parsing validation code from xmlFreeTextReader() to
      fix a second use-after-free when calling xmlTextReaderClose()
      before xmlFreeTextReader().  This regressed in v2.9.10 with
      commit 57a3af56f.

 fuzz/xml.c  |  7 +++++++
 xmlreader.c | 40 ++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 22 deletions(-)
---
diff --git a/fuzz/xml.c b/fuzz/xml.c
index 8b4c4efc..87a9fc55 100644
--- a/fuzz/xml.c
+++ b/fuzz/xml.c
@@ -29,6 +29,7 @@ int
 LLVMFuzzerTestOneInput(const char *data, size_t size) {
     static const size_t maxChunkSize = 128;
     xmlDocPtr doc;
+    xmlDocPtr preservedReaderDoc = NULL;
     xmlParserCtxtPtr ctxt;
     xmlTextReaderPtr reader;
     xmlChar *out;
@@ -94,7 +95,13 @@ LLVMFuzzerTestOneInput(const char *data, size_t size) {
             }
         }
     }
+    if (size % 5 == 0)
+        preservedReaderDoc = xmlTextReaderCurrentDoc(reader);
+    if (size % 3 == 0)
+        xmlTextReaderClose(reader);
     xmlFreeTextReader(reader);
+    if (size % 5 == 0 && preservedReaderDoc)
+        xmlFreeDoc(preservedReaderDoc);
 
 exit:
     xmlFuzzDataCleanup();
diff --git a/xmlreader.c b/xmlreader.c
index ba95813b..b3299ced 100644
--- a/xmlreader.c
+++ b/xmlreader.c
@@ -2181,36 +2181,16 @@ xmlFreeTextReader(xmlTextReaderPtr reader) {
        xmlFree(reader->patternTab);
     }
 #endif
-    if (reader->faketext != NULL) {
-       xmlFreeNode(reader->faketext);
-    }
+    if (reader->mode != XML_TEXTREADER_MODE_CLOSED)
+        xmlTextReaderClose(reader);
     if (reader->ctxt != NULL) {
         if (reader->dict == reader->ctxt->dict)
            reader->dict = NULL;
-#ifdef LIBXML_VALID_ENABLED
-       if ((reader->ctxt->vctxt.vstateTab != NULL) &&
-           (reader->ctxt->vctxt.vstateMax > 0)){
-#ifdef LIBXML_REGEXP_ENABLED
-            while (reader->ctxt->vctxt.vstateNr > 0)
-                xmlValidatePopElement(&reader->ctxt->vctxt, NULL, NULL, NULL);
-#endif /* LIBXML_REGEXP_ENABLED */
-           xmlFree(reader->ctxt->vctxt.vstateTab);
-           reader->ctxt->vctxt.vstateTab = NULL;
-           reader->ctxt->vctxt.vstateMax = 0;
-       }
-#endif /* LIBXML_VALID_ENABLED */
-       if (reader->ctxt->myDoc != NULL) {
-           if (reader->preserve == 0)
-               xmlTextReaderFreeDoc(reader, reader->ctxt->myDoc);
-           reader->ctxt->myDoc = NULL;
-       }
        if (reader->allocs & XML_TEXTREADER_CTXT)
            xmlFreeParserCtxt(reader->ctxt);
     }
     if (reader->sax != NULL)
        xmlFree(reader->sax);
-    if ((reader->input != NULL)  && (reader->allocs & XML_TEXTREADER_INPUT))
-       xmlFreeParserInputBuffer(reader->input);
     if (reader->buffer != NULL)
         xmlBufFree(reader->buffer);
     if (reader->entTab != NULL)
@@ -2241,7 +2221,23 @@ xmlTextReaderClose(xmlTextReaderPtr reader) {
     reader->node = NULL;
     reader->curnode = NULL;
     reader->mode = XML_TEXTREADER_MODE_CLOSED;
+    if (reader->faketext != NULL) {
+        xmlFreeNode(reader->faketext);
+        reader->faketext = NULL;
+    }
     if (reader->ctxt != NULL) {
+#ifdef LIBXML_VALID_ENABLED
+       if ((reader->ctxt->vctxt.vstateTab != NULL) &&
+           (reader->ctxt->vctxt.vstateMax > 0)){
+#ifdef LIBXML_REGEXP_ENABLED
+            while (reader->ctxt->vctxt.vstateNr > 0)
+                xmlValidatePopElement(&reader->ctxt->vctxt, NULL, NULL, NULL);
+#endif /* LIBXML_REGEXP_ENABLED */
+           xmlFree(reader->ctxt->vctxt.vstateTab);
+           reader->ctxt->vctxt.vstateTab = NULL;
+           reader->ctxt->vctxt.vstateMax = 0;
+       }
+#endif /* LIBXML_VALID_ENABLED */
        xmlStopParser(reader->ctxt);
        if (reader->ctxt->myDoc != NULL) {
            if (reader->preserve == 0)