fix(security): prevent trezor-connect-src connectSrc-override XSS (Immunefi #40463)

apps/frontend/src/index.tsx

@@ -29,21 +29,29 @@ import './locales/i18n';
 import { router } from './router';
 import './styles/tailwindcss/index.css';
 import { rskClient } from './utils/clients';
+import { stripTrezorConnectSrcParam } from './utils/sanitizeUrl';
 
-const checkAndRemoveQueryParam = () => {
-  const urlParams = new URLSearchParams(window.location.search);
+// Neutralise the `@trezor/connect-web` `connectSrc`-override XSS (Immunefi #40463 and
+// its `trezor-connect-srcz` substring bypass) before anything can read the query
+// string. connect-web matches the override param by *substring*, so we must strip on
+// the same basis rather than by exact key — see stripTrezorConnectSrcParam(). This runs
+// synchronously at module load, well before the Trezor onboarding module ever calls
+// TrezorConnect.init() (which only happens on user wallet selection).
+const removeTrezorConnectSrcParam = () => {
+  const sanitized = stripTrezorConnectSrcParam(window.location.search);
 
-  if (urlParams.has('trezor-connect-src')) {
-    urlParams.delete('trezor-connect-src');
-
-    const newUrl = `${window.location.pathname}?${urlParams.toString()}`;
+  if (sanitized !== null) {
+    const newUrl =
+      window.location.pathname +
+      (sanitized ? `?${sanitized}` : '') +
+      window.location.hash;
 
     window.history.replaceState({}, document.title, newUrl);
     window.location.reload();
   }
 };
 
-checkAndRemoveQueryParam();
+removeTrezorConnectSrcParam();
 
 const root = ReactDOM.createRoot(
   document.getElementById('root') as HTMLElement,

apps/frontend/src/utils/sanitizeUrl.test.ts

@@ -0,0 +1,61 @@
+import { stripTrezorConnectSrcParam } from './sanitizeUrl';
+
+describe('utils/sanitizeUrl.ts', () => {
+  describe('stripTrezorConnectSrcParam()', () => {
+    // @trezor/connect-web's parseConnectSettings picks up `connectSrc` from ANY
+    // '&'-delimited query segment whose text *contains* the substring
+    // "trezor-connect-src" (`v.indexOf(...) >= 0`), then uses its value verbatim as
+    // the popup/iframe URL with no scheme validation for non-http values — so a
+    // `javascript:` value executes in our origin (Immunefi #40463). These cases pin
+    // down that we strip on the SAME substring basis, not an exact key, so the
+    // `…-srcz` (and similar) bypasses cannot slip through.
+
+    it('strips the exact trezor-connect-src param', () => {
+      expect(
+        stripTrezorConnectSrcParam('?trezor-connect-src=javascript:alert(1)'),
+      ).toBe('');
+    });
+
+    it('strips the trailing-character bypass variant (trezor-connect-srcz)', () => {
+      expect(
+        stripTrezorConnectSrcParam(
+          '?trezor-connect-srcz=javascript://ex.com/%250aalert(document.domain)//',
+        ),
+      ).toBe('');
+    });
+
+    it('strips a leading-character variant (x-prefixed)', () => {
+      expect(
+        stripTrezorConnectSrcParam('?xtrezor-connect-src=javascript:alert(1)'),
+      ).toBe('');
+    });
+
+    it('strips case-insensitively (superset of connect-web’s case-sensitive match)', () => {
+      expect(
+        stripTrezorConnectSrcParam('?Trezor-Connect-Srcz=javascript:alert(1)'),
+      ).toBe('');
+    });
+
+    it('removes only the offending segment and preserves the rest', () => {
+      expect(
+        stripTrezorConnectSrcParam('?a=1&trezor-connect-srcz=evil&b=2'),
+      ).toBe('a=1&b=2');
+    });
+
+    it('removes every offending segment when several are present', () => {
+      expect(
+        stripTrezorConnectSrcParam(
+          '?trezor-connect-src=1&x-trezor-connect-src=2',
+        ),
+      ).toBe('');
+    });
+
+    it('returns null when no matching param is present', () => {
+      expect(stripTrezorConnectSrcParam('?a=1&b=2')).toBeNull();
+    });
+
+    it('returns null for an empty query string', () => {
+      expect(stripTrezorConnectSrcParam('')).toBeNull();
+    });
+  });
+});

apps/frontend/src/utils/sanitizeUrl.ts

@@ -0,0 +1,45 @@
+/**
+ * The substring `@trezor/connect-web` scans the query string for when deciding
+ * whether to override `connectSrc` (the popup/iframe origin). See
+ * `@trezor/connect-web`'s `parseConnectSettings`, which does
+ * `window.location.search.split('&').find(v => v.indexOf('trezor-connect-src') >= 0)`.
+ */
+export const TREZOR_CONNECT_SRC_NEEDLE = 'trezor-connect-src';
+
+/**
+ * Removes any query-string segment that `@trezor/connect-web` would treat as a
+ * `connectSrc` override.
+ *
+ * connect-web's `parseConnectSettings` scans `window.location.search.split('&')`
+ * and takes the first segment whose text *contains* the substring
+ * `trezor-connect-src` (`v.indexOf(...) >= 0`), then uses its value verbatim as the
+ * popup/iframe URL with no scheme validation for non-`http` values. A `javascript:`
+ * value therefore executes in our origin (Immunefi #40463).
+ *
+ * Because the library matches by substring, an exact-key filter
+ * (`URLSearchParams.has('trezor-connect-src')`) is trivially bypassed by variants
+ * such as `trezor-connect-srcz` or `x-trezor-connect-src`. We strip on the same
+ * substring basis instead — case-insensitively, a superset of connect-web's
+ * case-sensitive match — so no variant survives.
+ *
+ * @param search Raw `window.location.search` (with or without a leading `?`).
+ * @returns The rebuilt query string WITHOUT a leading `?` (an empty string when
+ *   every segment was stripped), or `null` when nothing matched and the URL should
+ *   be left untouched.
+ */
+export const stripTrezorConnectSrcParam = (search: string): string | null => {
+  if (!search) {
+    return null;
+  }
+
+  const segments = search.replace(/^\?/, '').split('&');
+  const kept = segments.filter(
+    segment => !segment.toLowerCase().includes(TREZOR_CONNECT_SRC_NEEDLE),
+  );
+
+  if (kept.length === segments.length) {
+    return null;
+  }
+
+  return kept.join('&');
+};