From 99eae21fada0d541efdb221de803f76d15eed054 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 4 Jul 2024 11:45:35 -0400 Subject: [PATCH] Add FIXMEs for tsec violations --- packages/auth/src/platform_browser/index.ts | 4 ++++ packages/auth/src/platform_browser/load_js.test.ts | 4 ++++ packages/database/src/realtime/BrowserPollConnection.ts | 7 ++++++- packages/messaging/src/helpers/registerDefaultSw.ts | 2 +- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/platform_browser/index.ts b/packages/auth/src/platform_browser/index.ts index f94525bfeb7..3dbf05714ef 100644 --- a/packages/auth/src/platform_browser/index.ts +++ b/packages/auth/src/platform_browser/index.ts @@ -124,6 +124,10 @@ _setExternalJSProvider({ // TODO: consider adding timeout support & cancellation return new Promise((resolve, reject) => { const el = document.createElement('script'); + // Do not use setAttribute, since it can lead to XSS. Instead, use the safevalues library to + // safely set an attribute for a sanitized trustedResourceUrl. Since the trustedResourceUrl + // must be initialized from a template string literal, this could involve some heavy + // refactoring. el.setAttribute('src', url); el.onload = resolve; el.onerror = e => { diff --git a/packages/auth/src/platform_browser/load_js.test.ts b/packages/auth/src/platform_browser/load_js.test.ts index 972fb292065..d7535b8eb82 100644 --- a/packages/auth/src/platform_browser/load_js.test.ts +++ b/packages/auth/src/platform_browser/load_js.test.ts @@ -44,6 +44,8 @@ describe('platform-browser/load_js', () => { loadJS(url: string): Promise { return new Promise((resolve, reject) => { const el = document.createElement('script'); + // FIXME: Do not use setAttribute, as this can lead to XSS. Instead, use the safevalues + // library, or get an exception for tests. el.setAttribute('src', url); el.onload = resolve; el.onerror = e => { @@ -65,6 +67,8 @@ describe('platform-browser/load_js', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises _loadJS('http://localhost/url'); + // FIXME: Do not use setAttribute, as this can lead to XSS. Instead, use the safevalues + // library, or get an exception for tests. expect(el.setAttribute).to.have.been.calledWith( 'src', 'http://localhost/url' diff --git a/packages/database/src/realtime/BrowserPollConnection.ts b/packages/database/src/realtime/BrowserPollConnection.ts index d8c32665721..da56c414246 100644 --- a/packages/database/src/realtime/BrowserPollConnection.ts +++ b/packages/database/src/realtime/BrowserPollConnection.ts @@ -475,7 +475,8 @@ export class FirebaseIFrameScriptHolder { const iframeContents = '' + script + ''; try { this.myIFrame.doc.open(); - // FIXME: Use the safevalues library to sanitize this + // FIXME: Do not use document.write, since it can lead to XSS. Instead, use the safevalues + // library to sanitize the HTML in the iframeContents. this.myIFrame.doc.write(iframeContents); this.myIFrame.doc.close(); } catch (e) { @@ -718,6 +719,10 @@ export class FirebaseIFrameScriptHolder { const newScript = this.myIFrame.doc.createElement('script'); newScript.type = 'text/javascript'; newScript.async = true; + // FIXME: We cannot assign an arbitrary URL to a script attached to the DOM, since it is + // at risk of XSS. We should use the safevalues library to create a safeScriptEl, and + // assign a sanitized trustedResourceURL to it. Since the URL must be a template string + // literal, this could require some heavy refactoring. newScript.src = url; // eslint-disable-next-line @typescript-eslint/no-explicit-any newScript.onload = (newScript as any).onreadystatechange = diff --git a/packages/messaging/src/helpers/registerDefaultSw.ts b/packages/messaging/src/helpers/registerDefaultSw.ts index 3ab4dd870f5..63bccc7acb2 100644 --- a/packages/messaging/src/helpers/registerDefaultSw.ts +++ b/packages/messaging/src/helpers/registerDefaultSw.ts @@ -24,7 +24,7 @@ export async function registerDefaultSw( messaging: MessagingService ): Promise { try { - // FIXME: Use safevalues to register the service worker with a sanitized URL. + // FIXME: Use safevalues to register the service worker with a sanitized trustedResourceUrl. messaging.swRegistration = await navigator.serviceWorker.register( DEFAULT_SW_PATH, {