[libxslt] Fix use-after-free in xsltApplyTemplates



commit 50f9c9cd3b7dfe9b3c8c795247752d1fdcadcac8
Author: Nick Wellnhofer <wellnhofer aevum de>
Date:   Sat Jun 12 20:02:53 2021 +0200

    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.

 libxslt/transform.c | 119 ++++------------------------------------------------
 1 file changed, 7 insertions(+), 112 deletions(-)
---
diff --git a/libxslt/transform.c b/libxslt/transform.c
index 04522154..3aba354f 100644
--- a/libxslt/transform.c
+++ b/libxslt/transform.c
@@ -1895,7 +1895,7 @@ static void
 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 @@ xsltDefaultProcessOneNode(xsltTransformContextPtr ctxt, xmlNodePtr node,
            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 @@ xsltApplyTemplates(xsltTransformContextPtr ctxt, xmlNodePtr node,
     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 @@ xsltApplyTemplates(xsltTransformContextPtr ctxt, xmlNodePtr node,
        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;
-           }
        }
     }
 


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