# HG changeset patch # User Nick Wellnhofer <wellnhofer@aevum.de> # Date 1623520973 -7200 # Sat Jun 12 20:02:53 2021 +0200 # Node ID a64262f72b82862c74777ac543ce24ccc153f01d # Parent 71c854a7a5b680d3e133079dc450d60c5394669c Fix use-after-free in xsltApplyTemplates xsltApplyTemplates without a select expression could delete nodes in the source document. 1. Text nodes with strippable whitespace Whitespace from input documents is already stripped, so there's no need to strip it again. Under certain circumstances, xsltApplyTemplates could be fooled into deleting text nodes that are still referenced, resulting in a use-after-free. 2. The DTD The DTD was only unlinked, but there's no good reason to do this just now. Maybe it was meant as a micro-optimization. 3. Unknown nodes Useless and dangerous as well, especially with XInclude nodes. See https://gitlab.gnome.org/GNOME/libxml2/-/issues/268 Simply stop trying to uselessly delete nodes when applying a template. This part of the code is probably a leftover from a time where xsltApplyStripSpaces wasn't implemented yet. Also note that xsltApplyTemplates with a select expression never tried to delete nodes. Also stop xsltDefaultProcessOneNode from deleting nodes for the same reasons. This fixes CVE-2021-30560. diff --git a/libxslt/transform.c b/libxslt/transform.c --- a/libxslt/transform.c +++ b/libxslt/transform.c @@ -1895,7 +1895,7 @@ xsltDefaultProcessOneNode(xsltTransformContextPtr ctxt, xmlNodePtr node, xsltStackElemPtr params) { xmlNodePtr copy; - xmlNodePtr delete = NULL, cur; + xmlNodePtr cur; int nbchild = 0, oldSize; int childno = 0, oldPos; xsltTemplatePtr template; @@ -1968,54 +1968,13 @@ return; } /* - * Handling of Elements: first pass, cleanup and counting + * Handling of Elements: first pass, counting */ cur = node->children; while (cur != NULL) { - switch (cur->type) { - case XML_TEXT_NODE: - case XML_CDATA_SECTION_NODE: - case XML_DOCUMENT_NODE: - case XML_HTML_DOCUMENT_NODE: - case XML_ELEMENT_NODE: - case XML_PI_NODE: - case XML_COMMENT_NODE: - nbchild++; - break; - case XML_DTD_NODE: - /* Unlink the DTD, it's still reachable using doc->intSubset */ - if (cur->next != NULL) - cur->next->prev = cur->prev; - if (cur->prev != NULL) - cur->prev->next = cur->next; - break; - default: -#ifdef WITH_XSLT_DEBUG_PROCESS - XSLT_TRACE(ctxt,XSLT_TRACE_PROCESS_NODE,xsltGenericDebug(xsltGenericDebugContext, - "xsltDefaultProcessOneNode: skipping node type %d\n", - cur->type)); -#endif - delete = cur; - } + if (IS_XSLT_REAL_NODE(cur)) + nbchild++; cur = cur->next; - if (delete != NULL) { -#ifdef WITH_XSLT_DEBUG_PROCESS - XSLT_TRACE(ctxt,XSLT_TRACE_PROCESS_NODE,xsltGenericDebug(xsltGenericDebugContext, - "xsltDefaultProcessOneNode: removing ignorable blank node\n")); -#endif - xmlUnlinkNode(delete); - xmlFreeNode(delete); - delete = NULL; - } - } - if (delete != NULL) { -#ifdef WITH_XSLT_DEBUG_PROCESS - XSLT_TRACE(ctxt,XSLT_TRACE_PROCESS_NODE,xsltGenericDebug(xsltGenericDebugContext, - "xsltDefaultProcessOneNode: removing ignorable blank node\n")); -#endif - xmlUnlinkNode(delete); - xmlFreeNode(delete); - delete = NULL; } /* @@ -4864,7 +4823,7 @@ xsltStylePreCompPtr comp = (xsltStylePreCompPtr) castedComp; #endif int i; - xmlNodePtr cur, delNode = NULL, oldContextNode; + xmlNodePtr cur, oldContextNode; xmlNodeSetPtr list = NULL, oldList; xsltStackElemPtr withParams = NULL; int oldXPProximityPosition, oldXPContextSize; @@ -4998,73 +4957,9 @@ else cur = NULL; while (cur != NULL) { - switch (cur->type) { - case XML_TEXT_NODE: - if ((IS_BLANK_NODE(cur)) && - (cur->parent != NULL) && - (cur->parent->type == XML_ELEMENT_NODE) && - (ctxt->style->stripSpaces != NULL)) { - const xmlChar *val; - - if (cur->parent->ns != NULL) { - val = (const xmlChar *) - xmlHashLookup2(ctxt->style->stripSpaces, - cur->parent->name, - cur->parent->ns->href); - if (val == NULL) { - val = (const xmlChar *) - xmlHashLookup2(ctxt->style->stripSpaces, - BAD_CAST "*", - cur->parent->ns->href); - } - } else { - val = (const xmlChar *) - xmlHashLookup2(ctxt->style->stripSpaces, - cur->parent->name, NULL); - } - if ((val != NULL) && - (xmlStrEqual(val, (xmlChar *) "strip"))) { - delNode = cur; - break; - } - } - /* Intentional fall-through */ - case XML_ELEMENT_NODE: - case XML_DOCUMENT_NODE: - case XML_HTML_DOCUMENT_NODE: - case XML_CDATA_SECTION_NODE: - case XML_PI_NODE: - case XML_COMMENT_NODE: - xmlXPathNodeSetAddUnique(list, cur); - break; - case XML_DTD_NODE: - /* Unlink the DTD, it's still reachable - * using doc->intSubset */ - if (cur->next != NULL) - cur->next->prev = cur->prev; - if (cur->prev != NULL) - cur->prev->next = cur->next; - break; - case XML_NAMESPACE_DECL: - break; - default: -#ifdef WITH_XSLT_DEBUG_PROCESS - XSLT_TRACE(ctxt,XSLT_TRACE_APPLY_TEMPLATES,xsltGenericDebug(xsltGenericDebugContext, - "xsltApplyTemplates: skipping cur type %d\n", - cur->type)); -#endif - delNode = cur; - } + if (IS_XSLT_REAL_NODE(cur)) + xmlXPathNodeSetAddUnique(list, cur); cur = cur->next; - if (delNode != NULL) { -#ifdef WITH_XSLT_DEBUG_PROCESS - XSLT_TRACE(ctxt,XSLT_TRACE_APPLY_TEMPLATES,xsltGenericDebug(xsltGenericDebugContext, - "xsltApplyTemplates: removing ignorable blank cur\n")); -#endif - xmlUnlinkNode(delNode); - xmlFreeNode(delNode); - delNode = NULL; - } } }