Add FIXMEs for tsec violations

firebase · Jul 4, 2024 · 99eae21 · 99eae21
1 parent be0bb23
commit 99eae21
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 2 deletions.
diff --git 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
@@ -44,6 +44,8 @@ describe('platform-browser/load_js', () => {
         loadJS(url: string): Promise<Event> {
           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
@@ -475,7 +475,8 @@ export class FirebaseIFrameScriptHolder {
       const iframeContents = '<html><body>' + script + '</body></html>';
       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
@@ -24,7 +24,7 @@ export async function registerDefaultSw(
   messaging: MessagingService
 ): Promise<void> {
   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,
       {