summaryrefslogtreecommitdiff
path: root/tree.c
diff options
context:
space:
mode:
authorDavid Kilzer <ddkilzer@apple.com>2022-03-19 17:17:40 -0700
committerDavid Kilzer <ddkilzer@webkit.org>2022-05-25 16:55:26 +0000
commit4bc3ebf3eaba352fbbce2ef70ad00a3c7752478a (patch)
treebe469b4d1640c728b2da05df0895f91005404f7e /tree.c
parent0aa8652e596a20e95ed334ac65cf15e6e9ec4b3b (diff)
downloadlibxml2-4bc3ebf3eaba352fbbce2ef70ad00a3c7752478a.tar.gz
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.
Diffstat (limited to 'tree.c')
-rw-r--r--tree.c27
1 files changed, 26 insertions, 1 deletions
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;
}
}