fix(sanitization): match onload case- and space-insensitively

core/src/components/loading/test/loading.spec.ts

@@ -4,6 +4,18 @@ import { config } from '../../../global/config';
 import { Loading } from '../loading';
 
 describe('loading: custom html', () => {
+  let consoleWarnSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    consoleWarnSpy = jest.spyOn(console, 'warn');
+    // Suppress console.warn output from polluting the test output
+    consoleWarnSpy.mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    consoleWarnSpy.mockRestore();
+  });
+
   it('should not allow for custom html by default', async () => {
     const page = await newSpecPage({
       components: [Loading],
@@ -15,6 +27,17 @@ describe('loading: custom html', () => {
     expect(content.querySelector('button.custom-html')).toBe(null);
   });
 
+  it('should discard a message carrying an onload handler', async () => {
+    config.reset({ innerHTMLTemplatesEnabled: true });
+    const page = await newSpecPage({
+      components: [Loading],
+      html: `<ion-loading message="<svg><svg onLoad='alert(1)'></svg></svg>Custom Text"></ion-loading>`,
+    });
+
+    const content = page.body.querySelector('.loading-content')!;
+    expect(content.innerHTML).toEqual('');
+  });
+
   it('should allow for custom html', async () => {
     config.reset({ innerHTMLTemplatesEnabled: true });
     const page = await newSpecPage({

core/src/utils/sanitization/index.ts

@@ -1,4 +1,4 @@
-import { printIonError } from '@utils/logging';
+import { printIonError, printIonWarning } from '@utils/logging';
 
 /**
  * Sanitize an untrusted HTML string.
@@ -19,8 +19,8 @@ import { printIonError } from '@utils/logging';
  * @param untrustedString - The HTML string to sanitize. Pass an
  * `IonicSafeString` to bypass sanitization, or `undefined` to short-circuit.
  * @returns The sanitized HTML string, or `undefined` if the input was
- * `undefined`. Returns `''` if sanitization fails or the input contains
- * an inline `onload=` handler.
+ * `undefined`. Returns `''` if sanitization fails or the input appears to
+ * carry an inline `onload` handler.
  */
 export const sanitizeDOMString = (untrustedString: IonicSafeString | string | undefined): string | undefined => {
   try {
@@ -32,12 +32,18 @@ export const sanitizeDOMString = (untrustedString: IonicSafeString | string | un
     }
 
     /**
-     * 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.
+     * In Blink a non-outermost `<svg>` (e.g. `<svg><svg onload=...>`) fires
+     * its load handler while `innerHTML` below parses the string, so it runs
+     * before the attribute-stripping pass can remove it. The match has to
+     * stay this loose because attribute names are case-insensitive and
+     * whitespace is allowed before the `=`.
      */
-    if (untrustedString.includes('onload=')) {
+    if (/onload\s*=/i.test(untrustedString)) {
+      printIonWarning(
+        'sanitizeDOMString - Content was discarded because it appears to contain an onload handler:',
+        untrustedString.substring(0, 100)
+      );
+
       return '';
     }
 

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

@@ -1,6 +1,18 @@
 import { blockedTags, IonicSafeString, reflectPropertiesToAttributes, sanitizeDOMString, sanitizeDOMTree } from '..';
 
 describe('sanitizeDOMString', () => {
+  let consoleWarnSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    consoleWarnSpy = jest.spyOn(console, 'warn');
+    // Suppress console.warn output from polluting the test output
+    consoleWarnSpy.mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    consoleWarnSpy.mockRestore();
+  });
+
   it('disable sanitizer', () => {
     enableSanitizer(false);
     expect(sanitizeDOMString('<img src="x" onerror="alert(document.cookie);">')).toEqual(
@@ -26,6 +38,33 @@ describe('sanitizeDOMString', () => {
     ).toEqual('<button id="myButton" name="myButton">harmless button</button>');
   });
 
+  it('filter onload', () => {
+    /**
+     * Only a non-outermost `<svg>` fires its load handler mid-parse, so the
+     * nesting is what makes these payloads real.
+     */
+    expect(sanitizeDOMString('<svg><svg onload=alert(document.cookie)></svg></svg>')).toEqual('');
+    expect(sanitizeDOMString('<svg><svg onLoad=alert(document.cookie)></svg></svg>')).toEqual('');
+    expect(sanitizeDOMString('<svg><svg ONLOAD=alert(document.cookie)></svg></svg>')).toEqual('');
+    expect(sanitizeDOMString('<svg><svg onload =alert(document.cookie)></svg></svg>')).toEqual('');
+  });
+
+  it('filter onload discards benign content too (known false positive)', () => {
+    /**
+     * The check runs before parsing, so it can't tell an attribute from a
+     * URL or from prose. Both of these lose their whole string.
+     */
+    expect(sanitizeDOMString('<a href="/docs?onload=1">docs</a>')).toEqual('');
+    expect(sanitizeDOMString('<a href="/docs?onLoad=1">docs</a>')).toEqual('');
+  });
+
+  it('warn when content is discarded by the onload check', () => {
+    sanitizeDOMString('<svg><svg onLoad=alert(document.cookie)></svg></svg>');
+
+    expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+    expect(consoleWarnSpy.mock.calls[0][0]).toContain('onload handler');
+  });
+
   it('filter <a> href JS', () => {
     expect(sanitizeDOMString('<a href="javascript:alert(document.cookie)">harmless link</a>')).toEqual(
       '<a>harmless link</a>'