fix(sanitizer): improve reliability of sanitizer (#26820)

core/src/utils/sanitization/index.ts

@@ -12,6 +12,16 @@ export const sanitizeDOMString = (untrustedString: IonicSafeString | string | un
       return untrustedString;
     }
 
+    /**
+     * onload is fired when appending to a document
+     * fragment in Chrome. If a string
+     * contains onload then we should not
+     * attempt to add this to the fragment.
+     */
+    if (untrustedString.includes('onload=')) {
+      return '';
+    }
+
     /**
      * Create a document fragment
      * separate from the main DOM,
@@ -89,6 +99,17 @@ const sanitizeElement = (element: any) => {
     return;
   }
 
+  /**
+   * If attributes is not a NamedNodeMap
+   * then we should remove the element entirely.
+   * This helps avoid DOM Clobbering attacks where
+   * attributes is overridden.
+   */
+  if (typeof NamedNodeMap !== 'undefined' && !(element.attributes instanceof NamedNodeMap)) {
+    element.remove();
+    return;
+  }
+
   for (let i = element.attributes.length - 1; i >= 0; i--) {
     const attribute = element.attributes.item(i);
     const attributeName = attribute.name;
@@ -103,10 +124,21 @@ const sanitizeElement = (element: any) => {
     // that attempt to do any JS funny-business
     const attributeValue = attribute.value;
 
-    /* eslint-disable-next-line */
-    if (attributeValue != null && attributeValue.toLowerCase().includes('javascript:')) {
+    /**
+     * We also need to check the property value
+     * as javascript: can allow special characters
+     * such as 	 and still be valid (i.e. java	script)
+     */
+    const propertyValue = element[attributeName];
+
+    /* eslint-disable */
+    if (
+      (attributeValue != null && attributeValue.toLowerCase().includes('javascript:')) ||
+      (propertyValue != null && propertyValue.toLowerCase().includes('javascript:'))
+    ) {
       element.removeAttribute(attributeName);
     }
+    /* eslint-enable */
   }
 
   /**

core/src/utils/sanitization/test/sanitization.spec.ts

@@ -30,6 +30,9 @@ describe('sanitizeDOMString', () => {
     expect(sanitizeDOMString('<a href="javascript:alert(document.cookie)">harmless link</a>')).toEqual(
       '<a>harmless link</a>'
     );
+    expect(sanitizeDOMString('<a href="javascr&Tab;ipt:alert(document.cookie)">harmless link</a>')).toEqual(
+      '<a>harmless link</a>'
+    );
   });
 
   it('filter <a> href JS + class attribute', () => {