diff options
author | David Kilzer <ddkilzer@apple.com> | 2022-03-19 17:17:40 -0700 |
---|---|---|
committer | David Kilzer <ddkilzer@webkit.org> | 2022-05-25 16:55:26 +0000 |
commit | 4bc3ebf3eaba352fbbce2ef70ad00a3c7752478a (patch) | |
tree | be469b4d1640c728b2da05df0895f91005404f7e | |
parent | 0aa8652e596a20e95ed334ac65cf15e6e9ec4b3b (diff) | |
download | libxml2-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.
-rw-r--r-- | tree.c | 27 |
1 files changed, 26 insertions, 1 deletions
@@ -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; } } |