From 5e41391ed2585072095f42f7a6d40497f0e129d2 Mon Sep 17 00:00:00 2001
From: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Date: Fri, 17 Feb 2023 15:28:01 -0500
Subject: [PATCH] fix(sanitizer): improve reliability of sanitizer (#26820)

---
 core/src/utils/sanitization/index.ts          | 36 +++++++++++++++++--
 .../sanitization/test/sanitization.spec.ts    |  3 ++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/core/src/utils/sanitization/index.ts b/core/src/utils/sanitization/index.ts
index b60a48cad1..8d8b2d7992 100644
--- a/core/src/utils/sanitization/index.ts
+++ b/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 &Tab; and still be valid (i.e. java&Tab;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 */
   }
 
   /**
diff --git a/core/src/utils/sanitization/test/sanitization.spec.ts b/core/src/utils/sanitization/test/sanitization.spec.ts
index 96f386db18..15aca07c8f 100644
--- a/core/src/utils/sanitization/test/sanitization.spec.ts
+++ b/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', () => {