fix: Rolling back changes from previous fixes, trying more aggressive…

… comment handling
cure53 · Mar 26, 2024 · fc3c781 · fc3c781
1 parent 8a0dcf8
commit fc3c781
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 67 deletions.
diff --git a/dist/purify.cjs.js b/dist/purify.cjs.js
diff --git a/dist/purify.cjs.js.map b/dist/purify.cjs.js.map
diff --git a/dist/purify.es.mjs b/dist/purify.es.mjs
@@ -919,7 +919,7 @@ function createDOMPurify() {
    * @return {Boolean} true if clobbered, false if safe
    */
   const _isClobbered = function _isClobbered(elm) {
-    return elm instanceof HTMLFormElement && (typeof elm.nodeName !== 'string' || typeof elm.textContent !== 'string' || typeof elm.removeChild !== 'function' || !(elm.attributes instanceof NamedNodeMap) || typeof elm.removeAttribute !== 'function' || typeof elm.setAttribute !== 'function' || typeof elm.namespaceURI !== 'string' || typeof elm.insertBefore !== 'function' || typeof elm.hasChildNodes !== 'function');
+    return elm instanceof HTMLFormElement && (typeof elm.nodeName !== 'string' || typeof elm.textContent !== 'string' || typeof elm.data !== 'string' || typeof elm.removeChild !== 'function' || !(elm.attributes instanceof NamedNodeMap) || typeof elm.removeAttribute !== 'function' || typeof elm.setAttribute !== 'function' || typeof elm.namespaceURI !== 'string' || typeof elm.insertBefore !== 'function' || typeof elm.hasChildNodes !== 'function');
   };
 
   /**
@@ -974,10 +974,6 @@ function createDOMPurify() {
     /* Now let's check the element's type and name */
     const tagName = transformCaseFunc(currentNode.nodeName);
 
-    /* Reliably map the parent node and child node(s) */
-    const parentNode = getParentNode(currentNode) || currentNode.parentNode;
-    const childNodes = getChildNodes(currentNode) || currentNode.childNodes;
-
     /* Execute a hook if present */
     _executeHook('uponSanitizeElement', currentNode, {
       tagName,
@@ -990,14 +986,14 @@ function createDOMPurify() {
       return true;
     }
 
-    /* Remove any ocurrence of processing instructions */
-    if (currentNode.nodeType === 7) {
+    /* Remove any ocurrence of possibly malicious comments */
+    if (currentNode.nodeType === 8 && regExpTest(/<[/\w]/g, currentNode.data)) {
       _forceRemove(currentNode);
       return true;
     }
 
-    /* Remove comment nodes from XML-ish content */
-    if (currentNode.nodeType === 8 && parentNode.namespaceURI !== HTML_NAMESPACE) {
+    /* Remove any ocurrence of processing instructions */
+    if (currentNode.nodeType === 7) {
       _forceRemove(currentNode);
       return true;
     }
@@ -1015,10 +1011,14 @@ function createDOMPurify() {
       }
 
       /* Keep content except for bad-listed elements */
-      if (KEEP_CONTENT && !FORBID_CONTENTS[tagName] && childNodes && parentNode) {
-        const childCount = childNodes.length;
-        for (let i = childCount - 1; i >= 0; --i) {
-          parentNode.insertBefore(cloneNode(childNodes[i], true), getNextSibling(currentNode));
+      if (KEEP_CONTENT && !FORBID_CONTENTS[tagName]) {
+        const parentNode = getParentNode(currentNode) || currentNode.parentNode;
+        const childNodes = getChildNodes(currentNode) || currentNode.childNodes;
+        if (childNodes && parentNode) {
+          const childCount = childNodes.length;
+          for (let i = childCount - 1; i >= 0; --i) {
+            parentNode.insertBefore(cloneNode(childNodes[i], true), getNextSibling(currentNode));
+          }
         }
       }
       _forceRemove(currentNode);

diff --git a/dist/purify.es.mjs.map b/dist/purify.es.mjs.map
diff --git a/dist/purify.js b/dist/purify.js
diff --git a/dist/purify.js.map b/dist/purify.js.map
diff --git a/dist/purify.min.js b/dist/purify.min.js
diff --git a/dist/purify.min.js.map b/dist/purify.min.js.map
diff --git a/src/purify.js b/src/purify.js
@@ -930,6 +930,7 @@ function createDOMPurify(window = getGlobal()) {
       elm instanceof HTMLFormElement &&
       (typeof elm.nodeName !== 'string' ||
         typeof elm.textContent !== 'string' ||
+        typeof elm.data !== 'string' ||
         typeof elm.removeChild !== 'function' ||
         !(elm.attributes instanceof NamedNodeMap) ||
         typeof elm.removeAttribute !== 'function' ||
@@ -993,10 +994,6 @@ function createDOMPurify(window = getGlobal()) {
     /* Now let's check the element's type and name */
     const tagName = transformCaseFunc(currentNode.nodeName);
 
-    /* Reliably map the parent node and child node(s) */
-    const parentNode = getParentNode(currentNode) || currentNode.parentNode;
-    const childNodes = getChildNodes(currentNode) || currentNode.childNodes;
-
     /* Execute a hook if present */
     _executeHook('uponSanitizeElement', currentNode, {
       tagName,
@@ -1014,17 +1011,14 @@ function createDOMPurify(window = getGlobal()) {
       return true;
     }
 
-    /* Remove any ocurrence of processing instructions */
-    if (currentNode.nodeType === 7) {
+    /* Remove any ocurrence of possibly malicious comments */
+    if (currentNode.nodeType === 8 && regExpTest(/<[/\w]/g, currentNode.data)) {
       _forceRemove(currentNode);
       return true;
     }
 
-    /* Remove comment nodes from XML-ish content */
-    if (
-      currentNode.nodeType === 8 &&
-      parentNode.namespaceURI !== HTML_NAMESPACE
-    ) {
+    /* Remove any ocurrence of processing instructions */
+    if (currentNode.nodeType === 7) {
       _forceRemove(currentNode);
       return true;
     }
@@ -1049,18 +1043,19 @@ function createDOMPurify(window = getGlobal()) {
       }
 
       /* Keep content except for bad-listed elements */
-      if (
-        KEEP_CONTENT &&
-        !FORBID_CONTENTS[tagName] &&
-        childNodes &&
-        parentNode
-      ) {
-        const childCount = childNodes.length;
-        for (let i = childCount - 1; i >= 0; --i) {
-          parentNode.insertBefore(
-            cloneNode(childNodes[i], true),
-            getNextSibling(currentNode)
-          );
+      if (KEEP_CONTENT && !FORBID_CONTENTS[tagName]) {
+        const parentNode = getParentNode(currentNode) || currentNode.parentNode;
+        const childNodes = getChildNodes(currentNode) || currentNode.childNodes;
+
+        if (childNodes && parentNode) {
+          const childCount = childNodes.length;
+
+          for (let i = childCount - 1; i >= 0; --i) {
+            parentNode.insertBefore(
+              cloneNode(childNodes[i], true),
+              getNextSibling(currentNode)
+            );
+          }
         }
       }