[libxml2/fix-ownership-of-xmlAttrPtr-name-in-xmlSetTreeDoc] Fix ownership of xmlNodePtr & xmlAttrPtr fields in xmlSetTreeDoc()




commit 138bd7130bec1e3eb9ffbc2c389b793686d65883
Author: David Kilzer <ddkilzer apple com>
Date:   Sat Mar 19 17:17:40 2022 -0700

    Fix ownership of xmlNodePtr & xmlAttrPtr fields in xmlSetTreeDoc()
    
    When changing `doc` on an xmlNodePtr or xmlAttrPtr, certain
    fields must either be a free-standing string, or they must be
    owned by `doc->dict`.
    
    The code to make this change was simply missing, so the crash
    happened when an xmlAttrPtr was being torn down after `doc`
    changed from non-NULL to NULL, but the `name` field was not
    copied.  This is scenario 1 below.
    
    The xmlNodePtr->name and xmlNodePtr->content fields are also
    fixed at the same time.  Note that xmlNodePtr->content is never
    added to the dictionary, so NULL is used instead of `newDict` to
    force a free-standing copy.
    
    This change covers all cases of dictionary changes:
    1. Owned by old dictionary -> NULL new dictionary
       - Create free-standing copy of string.
    2. Owned by old dictionary -> Non-NULL new dictionary
       - Get string from new dictionary pool.
    3. Not owned by old dictionary -> Non-NULL new dictionary
       - No action necessary (already a free-standing string).
    4. Not owned by old dictionary -> NULL new dictionary
       - No action necessary (already a free-standing string).
    
    * tree.c:
    (_copyStringForNewDictIfNeeded): Add.
    (xmlSetTreeDoc):
    - Update xmlNodePtr->name, xmlNodePtr->content and
      xmlAttrPtr->name when changing the document, if needed.
    
    Found by OSS-Fuzz Issue 45132.

 tree.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
---
diff --git a/tree.c b/tree.c
index 689a6b66..99eef30e 100644
--- a/tree.c
+++ b/tree.c
@@ -2812,6 +2812,20 @@ xmlNewDocComment(xmlDocPtr doc, const xmlChar *content) {
     return(cur);
 }
 
+static const xmlChar *_copyStringForNewDictIfNeeded(xmlDictPtr oldDict, xmlDictPtr newDict, const xmlChar 
*oldValue) {
+    const xmlChar *newValue = oldValue;
+    if (oldValue) {
+        int oldDictOwnsOldValue = oldDict && (xmlDictOwns(oldDict, oldValue) == 1);
+        if (oldDictOwnsOldValue) {
+            if (newDict)
+                newValue = xmlDictLookup(newDict, oldValue, -1);
+            else
+                newValue = xmlStrdup(oldValue);
+        }
+    }
+    return newValue;
+}
+
 /**
  * xmlSetTreeDoc:
  * @tree:  the top element
@@ -2826,6 +2840,9 @@ xmlSetTreeDoc(xmlNodePtr tree, xmlDocPtr doc) {
     if ((tree == NULL) || (tree->type == XML_NAMESPACE_DECL))
        return;
     if (tree->doc != doc) {
+        xmlDictPtr oldTreeDict = tree->doc ? tree->doc->dict : NULL;
+        xmlDictPtr newDict = doc ? doc->dict : NULL;
+
        if(tree->type == XML_ELEMENT_NODE) {
            prop = tree->properties;
            while (prop != NULL) {
@@ -2833,7 +2850,11 @@ xmlSetTreeDoc(xmlNodePtr tree, xmlDocPtr doc) {
                     xmlRemoveID(tree->doc, prop);
                 }
 
-               prop->doc = doc;
+                if (prop->doc != doc) {
+                    xmlDictPtr oldPropDict = prop->doc ? prop->doc->dict : NULL;
+                    prop->name = _copyStringForNewDictIfNeeded(oldPropDict, newDict, prop->name);
+                    prop->doc = doc;
+                }
                xmlSetListDoc(prop->children, doc);
 
                 /*
@@ -2862,6 +2883,10 @@ xmlSetTreeDoc(xmlNodePtr tree, xmlDocPtr doc) {
         } else if (tree->children != NULL) {
            xmlSetListDoc(tree->children, doc);
         }
+
+        tree->name = _copyStringForNewDictIfNeeded(oldTreeDict, newDict, tree->name);
+        tree->content = (xmlChar *)_copyStringForNewDictIfNeeded(oldTreeDict, NULL, tree->content);
+        /* FIXME: tree->ns should be updated as in xmlStaticCopyNode(). */
        tree->doc = doc;
     }
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]