1
0
mirror of https://gitlab.gnome.org/GNOME/libxml2.git synced 2025-01-15 23:24:06 +03:00

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.
This commit is contained in:
David Kilzer 2022-03-19 17:17:40 -07:00 committed by David Kilzer
parent 0aa8652e59
commit 4bc3ebf3ea

27
tree.c
View File

@ -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;
}
}