refactor(core): prevent infinite loops in clobbered elements check

packages/core/src/sanitization/html_sanitizer.ts

@@ -116,6 +116,7 @@ class SanitizingHtmlSerializer {
     // again, so it shouldn't be vulnerable to DOM clobbering.
     let current: Node = el.firstChild!;
     let traverseContent = true;
+    let parentNodes = [];
     while (current) {
       if (current.nodeType === Node.ELEMENT_NODE) {
         traverseContent = this.startElement(current as Element);
@@ -126,6 +127,7 @@ class SanitizingHtmlSerializer {
         this.sanitizedSomething = true;
       }
       if (traverseContent && current.firstChild) {
+        parentNodes.push(current);
         current = current.firstChild!;
         continue;
       }
@@ -135,14 +137,14 @@ class SanitizingHtmlSerializer {
           this.endElement(current as Element);
         }
 
-        let next = this.checkClobberedElement(current, current.nextSibling!);
+        let next = this.checkClobberedElement(current, getNextSibling(current)!);
 
         if (next) {
           current = next;
           break;
         }
 
-        current = this.checkClobberedElement(current, current.parentNode!);
+        current = this.checkClobberedElement(current, parentNodes.pop()!);
       }
     }
     return this.buf.join('');
@@ -157,7 +159,7 @@ class SanitizingHtmlSerializer {
    * @return True if the element's contents should be traversed.
    */
   private startElement(element: Element): boolean {
-    const tagName = element.nodeName.toLowerCase();
+    const tagName = getNodeName(element).toLowerCase();
     if (!VALID_ELEMENTS.hasOwnProperty(tagName)) {
       this.sanitizedSomething = true;
       return !SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS.hasOwnProperty(tagName);
@@ -183,7 +185,7 @@ class SanitizingHtmlSerializer {
   }
 
   private endElement(current: Element) {
-    const tagName = current.nodeName.toLowerCase();
+    const tagName = getNodeName(current).toLowerCase();
     if (VALID_ELEMENTS.hasOwnProperty(tagName) && !VOID_ELEMENTS.hasOwnProperty(tagName)) {
       this.buf.push('</');
       this.buf.push(tagName);
@@ -199,13 +201,37 @@ class SanitizingHtmlSerializer {
     if (nextNode &&
         (node.compareDocumentPosition(nextNode) &
          Node.DOCUMENT_POSITION_CONTAINED_BY) === Node.DOCUMENT_POSITION_CONTAINED_BY) {
-      throw new Error(`Failed to sanitize html because the element is clobbered: ${
-          (node as Element).outerHTML}`);
+      throw clobberedElementError(node);
     }
     return nextNode;
   }
 }
 
+/**
+ * Retrieves next sibling node and makes sure that there is no
+ * clobbering of the `nextSibling` property happening.
+ */
+function getNextSibling(node: Node): Node|null {
+  const nextSibling = node.nextSibling;
+  // Make sure there is no `nextSibling` clobbering.
+  if (nextSibling && node !== nextSibling.previousSibling) {
+    throw clobberedElementError(node);
+  }
+  return nextSibling;
+}
+
+/** Gets a reasonable nodeName, even for clobbered nodes. */
+export function getNodeName(node: Node): string {
+  const nodeName = node.nodeName;
+  // If the property is clobbered, assume it is an `HTMLFormElement`.
+  return (typeof nodeName === 'string') ? nodeName : 'FORM';
+}
+
+function clobberedElementError(node: Node) {
+  return new Error(
+      `Failed to sanitize html because the element is clobbered: ${(node as Element).outerHTML}`);
+}
+
 // Regular Expressions for parsing tags and attributes
 const SURROGATE_PAIR_REGEXP = /[\uD800-\uDBFF][\uDC00-\uDFFF]/g;
 // ! to ~ is the ASCII range.

packages/core/test/sanitization/html_sanitizer_spec.ts

@@ -206,18 +206,31 @@ describe('HTML sanitizer', () => {
     try {
       sanitizeHtml(defaultDoc, '<form><input name="parentNode" /></form>');
     } catch (e) {
-      // depending on the browser, we might ge an exception
+      // depending on the browser, we might get an exception
     }
     try {
       sanitizeHtml(defaultDoc, '<form><input name="nextSibling" /></form>');
     } catch (e) {
-      // depending on the browser, we might ge an exception
+      // depending on the browser, we might get an exception
     }
     try {
       sanitizeHtml(defaultDoc, '<form><div><div><input name="nextSibling" /></div></div></form>');
     } catch (e) {
-      // depending on the browser, we might ge an exception
+      // depending on the browser, we might get an exception
     }
+    try {
+      sanitizeHtml(defaultDoc, '<input name="nextSibling" form="a"><form id="a"></form>');
+    } catch (e) {
+      // depending on the browser, we might get an exception
+    }
+  });
+
+  fit('should properly identify node name if `nodeName` is clobbered', () => {
+    sanitizeHtml(defaultDoc, '<input name="nextSibling" form="a"><form id="a"></form>');
+
+    //     const output = sanitizeHtml(defaultDoc, '<form><input name=nodeName><input /></form>');
+    // console.log('--->', output);
+    expect('').toBe('');
   });
 
   // See