diff --git a/.changeset/tender-apes-clap.md b/.changeset/tender-apes-clap.md new file mode 100644 index 00000000000..d3629a245b8 --- /dev/null +++ b/.changeset/tender-apes-clap.md @@ -0,0 +1,6 @@ +--- +'@firebase/analytics': patch +'@firebase/app-check': patch +--- + +Revert introduction of safevalues to prevent issues from arising in Browser CommonJS environments due to ES5 incompatibility. For more information, see [GitHub PR #8395](https://github.com/firebase/firebase-js-sdk/pull/8395) diff --git a/packages/analytics/package.json b/packages/analytics/package.json index 1ece4e235d3..dca9ab5bea3 100644 --- a/packages/analytics/package.json +++ b/packages/analytics/package.json @@ -45,16 +45,15 @@ "@firebase/logger": "0.4.2", "@firebase/util": "1.9.7", "@firebase/component": "0.6.8", - "tslib": "^2.1.0", - "safevalues": "0.6.0" + "tslib": "^2.1.0" }, "license": "Apache-2.0", "devDependencies": { "@firebase/app": "0.10.7", + "rollup": "2.79.1", "@rollup/plugin-commonjs": "21.1.0", "@rollup/plugin-json": "4.1.0", "@rollup/plugin-node-resolve": "13.3.0", - "rollup": "2.79.1", "rollup-plugin-typescript2": "0.31.2", "typescript": "4.7.4" }, diff --git a/packages/analytics/src/helpers.test.ts b/packages/analytics/src/helpers.test.ts index 9145b625469..ff06ba3ea6c 100644 --- a/packages/analytics/src/helpers.test.ts +++ b/packages/analytics/src/helpers.test.ts @@ -24,12 +24,16 @@ import { insertScriptTag, wrapOrCreateGtag, findGtagScriptOnPage, - promiseAllSettled + promiseAllSettled, + createGtagTrustedTypesScriptURL, + createTrustedTypesPolicy } from './helpers'; -import { GtagCommand } from './constants'; +import { GtagCommand, GTAG_URL } from './constants'; import { Deferred } from '@firebase/util'; import { ConsentSettings } from './public-types'; import { removeGtagScripts } from '../testing/gtag-script-util'; +import { logger } from './logger'; +import { AnalyticsError, ERROR_FACTORY } from './errors'; const fakeMeasurementId = 'abcd-efgh-ijkl'; const fakeAppId = 'my-test-app-1234'; @@ -46,6 +50,71 @@ const fakeDynamicConfig: DynamicConfig = { }; const fakeDynamicConfigPromises = [Promise.resolve(fakeDynamicConfig)]; +describe('Trusted Types policies and functions', () => { + if (window.trustedTypes) { + describe('Trusted types exists', () => { + let ttStub: SinonStub; + + beforeEach(() => { + ttStub = stub( + window.trustedTypes as TrustedTypePolicyFactory, + 'createPolicy' + ).returns({ + createScriptURL: (s: string) => s + } as any); + }); + + afterEach(() => { + removeGtagScripts(); + ttStub.restore(); + }); + + it('Verify trustedTypes is called if the API is available', () => { + const trustedTypesPolicy = createTrustedTypesPolicy( + 'firebase-js-sdk-policy', + { + createScriptURL: createGtagTrustedTypesScriptURL + } + ); + + expect(ttStub).to.be.called; + expect(trustedTypesPolicy).not.to.be.undefined; + }); + + it('createGtagTrustedTypesScriptURL verifies gtag URL base exists when a URL is provided', () => { + expect(createGtagTrustedTypesScriptURL(GTAG_URL)).to.equal(GTAG_URL); + }); + + it('createGtagTrustedTypesScriptURL rejects URLs with non-gtag base', () => { + const NON_GTAG_URL = 'http://iamnotgtag.com'; + const loggerWarnStub = stub(logger, 'warn'); + const errorMessage = ERROR_FACTORY.create( + AnalyticsError.INVALID_GTAG_RESOURCE, + { + gtagURL: NON_GTAG_URL + } + ).message; + + expect(createGtagTrustedTypesScriptURL(NON_GTAG_URL)).to.equal(''); + expect(loggerWarnStub).to.be.calledWith(errorMessage); + }); + }); + } + describe('Trusted types does not exist', () => { + it('Verify trustedTypes functions are not called if the API is not available', () => { + delete window.trustedTypes; + const trustedTypesPolicy = createTrustedTypesPolicy( + 'firebase-js-sdk-policy', + { + createScriptURL: createGtagTrustedTypesScriptURL + } + ); + + expect(trustedTypesPolicy).to.be.undefined; + }); + }); +}); + describe('Gtag wrapping functions', () => { afterEach(() => { removeGtagScripts(); @@ -67,9 +136,8 @@ describe('Gtag wrapping functions', () => { insertScriptTag(customDataLayerName, fakeMeasurementId); const scriptTag = findGtagScriptOnPage(customDataLayerName); expect(scriptTag).to.not.be.null; - expect(scriptTag!.src).to.equal( - `https://www.googletagmanager.com/gtag/js?l=${customDataLayerName}&id=${fakeMeasurementId}` - ); + expect(scriptTag!.src).to.contain(`l=customDataLayerName`); + expect(scriptTag!.src).to.contain(`id=${fakeMeasurementId}`); }); // The test above essentially already touches this functionality but it is still valuable diff --git a/packages/analytics/src/helpers.ts b/packages/analytics/src/helpers.ts index 16c2fd78ed1..5ac24979115 100644 --- a/packages/analytics/src/helpers.ts +++ b/packages/analytics/src/helpers.ts @@ -24,12 +24,25 @@ import { import { DynamicConfig, DataLayer, Gtag, MinimalDynamicConfig } from './types'; import { GtagCommand, GTAG_URL } from './constants'; import { logger } from './logger'; -import { trustedResourceUrl } from 'safevalues'; -import { safeScriptEl } from 'safevalues/dom'; +import { AnalyticsError, ERROR_FACTORY } from './errors'; // Possible parameter types for gtag 'event' and 'config' commands type GtagConfigOrEventParams = ControlParams & EventParams & CustomParams; +/** + * Verifies and creates a TrustedScriptURL. + */ +export function createGtagTrustedTypesScriptURL(url: string): string { + if (!url.startsWith(GTAG_URL)) { + const err = ERROR_FACTORY.create(AnalyticsError.INVALID_GTAG_RESOURCE, { + gtagURL: url + }); + logger.warn(err.message); + return ''; + } + return url; +} + /** * Makeshift polyfill for Promise.allSettled(). Resolves when all promises * have either resolved or rejected. @@ -42,6 +55,29 @@ export function promiseAllSettled( return Promise.all(promises.map(promise => promise.catch(e => e))); } +/** + * Creates a TrustedTypePolicy object that implements the rules passed as policyOptions. + * + * @param policyName A string containing the name of the policy + * @param policyOptions Object containing implementations of instance methods for TrustedTypesPolicy, see {@link https://developer.mozilla.org/en-US/docs/Web/API/TrustedTypePolicy#instance_methods + * | the TrustedTypePolicy reference documentation}. + */ +export function createTrustedTypesPolicy( + policyName: string, + policyOptions: Partial +): Partial | undefined { + // Create a TrustedTypes policy that we can use for updating src + // properties + let trustedTypesPolicy: Partial | undefined; + if (window.trustedTypes) { + trustedTypesPolicy = window.trustedTypes.createPolicy( + policyName, + policyOptions + ); + } + return trustedTypesPolicy; +} + /** * Inserts gtag script tag into the page to asynchronously download gtag. * @param dataLayerName Name of datalayer (most often the default, "_dataLayer"). @@ -50,17 +86,21 @@ export function insertScriptTag( dataLayerName: string, measurementId: string ): void { - const script = document.createElement('script'); + const trustedTypesPolicy = createTrustedTypesPolicy( + 'firebase-js-sdk-policy', + { + createScriptURL: createGtagTrustedTypesScriptURL + } + ); + const script = document.createElement('script'); // We are not providing an analyticsId in the URL because it would trigger a `page_view` // without fid. We will initialize ga-id using gtag (config) command together with fid. - // - // We also have to ensure the template string before the first expression constitutes a valid URL - // start, as this is what the initial validation focuses on. If the template literal begins - // directly with an expression (e.g. `${GTAG_SCRIPT_URL}`), the validation fails due to an - // empty initial string. - const url = trustedResourceUrl`https://www.googletagmanager.com/gtag/js?l=${dataLayerName}&id=${measurementId}`; - safeScriptEl.setSrc(script, url); + + const gtagScriptURL = `${GTAG_URL}?l=${dataLayerName}&id=${measurementId}`; + (script.src as string | TrustedScriptURL) = trustedTypesPolicy + ? (trustedTypesPolicy as TrustedTypePolicy)?.createScriptURL(gtagScriptURL) + : gtagScriptURL; script.async = true; document.head.appendChild(script); diff --git a/packages/app-check/package.json b/packages/app-check/package.json index a80bc1f1137..c743c7dfebd 100644 --- a/packages/app-check/package.json +++ b/packages/app-check/package.json @@ -42,7 +42,6 @@ "@firebase/util": "1.9.7", "@firebase/component": "0.6.8", "@firebase/logger": "0.4.2", - "safevalues": "0.6.0", "tslib": "^2.1.0" }, "license": "Apache-2.0", diff --git a/packages/app-check/src/recaptcha.test.ts b/packages/app-check/src/recaptcha.test.ts index 03c72dcab73..7f028cca1a5 100644 --- a/packages/app-check/src/recaptcha.test.ts +++ b/packages/app-check/src/recaptcha.test.ts @@ -30,9 +30,7 @@ import { initializeV3, initializeEnterprise, getToken, - GreCAPTCHATopLevel, - RECAPTCHA_ENTERPRISE_URL, - RECAPTCHA_URL + GreCAPTCHATopLevel } from './recaptcha'; import * as utils from './util'; import { @@ -83,9 +81,7 @@ describe('recaptcha', () => { expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0); await initializeV3(app, FAKE_SITE_KEY); - const greCATPCHAScripts = findgreCAPTCHAScriptsOnPage(); - expect(greCATPCHAScripts.length).to.equal(1); - expect(greCATPCHAScripts[0].src).to.equal(RECAPTCHA_URL); + expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1); }); it('creates invisible widget', async () => { @@ -132,9 +128,7 @@ describe('recaptcha', () => { expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0); await initializeEnterprise(app, FAKE_SITE_KEY); - const greCAPTCHAScripts = findgreCAPTCHAScriptsOnPage(); - expect(greCAPTCHAScripts.length).to.equal(1); - expect(greCAPTCHAScripts[0].src).to.equal(RECAPTCHA_ENTERPRISE_URL); + expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1); }); it('creates invisible widget', async () => { diff --git a/packages/app-check/src/recaptcha.ts b/packages/app-check/src/recaptcha.ts index 0f0650296af..8eb72e2add7 100644 --- a/packages/app-check/src/recaptcha.ts +++ b/packages/app-check/src/recaptcha.ts @@ -19,12 +19,7 @@ import { FirebaseApp } from '@firebase/app'; import { getStateReference } from './state'; import { Deferred } from '@firebase/util'; import { getRecaptcha, ensureActivated } from './util'; -import { trustedResourceUrl } from 'safevalues'; -import { safeScriptEl } from 'safevalues/dom'; -// Note that these are used for testing. If they are changed, the URLs used in loadReCAPTCHAV3Script -// and loadReCAPTCHAEnterpriseScript must also be changed. They aren't used to create the URLs -// since trusted resource URLs must be created using template string literals. export const RECAPTCHA_URL = 'https://www.google.com/recaptcha/api.js'; export const RECAPTCHA_ENTERPRISE_URL = 'https://www.google.com/recaptcha/enterprise.js'; @@ -171,20 +166,14 @@ function renderInvisibleWidget( function loadReCAPTCHAV3Script(onload: () => void): void { const script = document.createElement('script'); - safeScriptEl.setSrc( - script, - trustedResourceUrl`https://www.google.com/recaptcha/api.js` - ); + script.src = RECAPTCHA_URL; script.onload = onload; document.head.appendChild(script); } function loadReCAPTCHAEnterpriseScript(onload: () => void): void { const script = document.createElement('script'); - safeScriptEl.setSrc( - script, - trustedResourceUrl`https://www.google.com/recaptcha/enterprise.js` - ); + script.src = RECAPTCHA_ENTERPRISE_URL; script.onload = onload; document.head.appendChild(script); } diff --git a/packages/auth/package.json b/packages/auth/package.json index 5d72e2a089f..e79b8f0102b 100644 --- a/packages/auth/package.json +++ b/packages/auth/package.json @@ -131,8 +131,7 @@ "@firebase/logger": "0.4.2", "@firebase/util": "1.9.7", "undici": "5.28.4", - "tslib": "^2.1.0", - "safevalues": "0.6.0" + "tslib": "^2.1.0" }, "license": "Apache-2.0", "devDependencies": { diff --git a/packages/auth/src/platform_browser/index.ts b/packages/auth/src/platform_browser/index.ts index f2682da38ec..f94525bfeb7 100644 --- a/packages/auth/src/platform_browser/index.ts +++ b/packages/auth/src/platform_browser/index.ts @@ -124,10 +124,6 @@ _setExternalJSProvider({ // TODO: consider adding timeout support & cancellation return new Promise((resolve, reject) => { const el = document.createElement('script'); - // TODO: 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 0fda2a6d016..972fb292065 100644 --- a/packages/auth/src/platform_browser/load_js.test.ts +++ b/packages/auth/src/platform_browser/load_js.test.ts @@ -44,8 +44,6 @@ describe('platform-browser/load_js', () => { loadJS(url: string): Promise { return new Promise((resolve, reject) => { const el = document.createElement('script'); - // TODO: 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 => { @@ -67,8 +65,6 @@ describe('platform-browser/load_js', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises _loadJS('http://localhost/url'); - // TODO: 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/auth/tsconfig.json b/packages/auth/tsconfig.json index f100c88aea0..03897eed09c 100644 --- a/packages/auth/tsconfig.json +++ b/packages/auth/tsconfig.json @@ -1,16 +1,10 @@ { "extends": "../../config/tsconfig.base.json", "compilerOptions": { - "outDir": "dist", - "plugins": [ - { - "name": "tsec", - "reportTsecDiagnosticsOnly": true, - } - ] + "outDir": "dist" }, "exclude": [ "dist/**/*", "demo/**/*" ] -} +} \ No newline at end of file diff --git a/packages/database-compat/tsconfig.json b/packages/database-compat/tsconfig.json index e2f493565c5..ce12ac3c5dc 100644 --- a/packages/database-compat/tsconfig.json +++ b/packages/database-compat/tsconfig.json @@ -3,15 +3,9 @@ "compilerOptions": { "outDir": "dist", "strict": false, - "downlevelIteration": true, - "plugins": [ - { - "name": "tsec", - "reportTsecDiagnosticsOnly": true, - } - ] + "downlevelIteration": true }, "exclude": [ "dist/**/*" ] -} +} \ No newline at end of file diff --git a/packages/database/package.json b/packages/database/package.json index 1e9d18a021d..c56f867c1eb 100644 --- a/packages/database/package.json +++ b/packages/database/package.json @@ -56,7 +56,6 @@ "@firebase/app-check-interop-types": "0.3.2", "@firebase/auth-interop-types": "0.2.3", "faye-websocket": "0.11.4", - "safevalues": "0.6.0", "tslib": "^2.1.0" }, "devDependencies": { diff --git a/packages/database/src/realtime/BrowserPollConnection.ts b/packages/database/src/realtime/BrowserPollConnection.ts index 48a567c8c33..697eef109c5 100644 --- a/packages/database/src/realtime/BrowserPollConnection.ts +++ b/packages/database/src/realtime/BrowserPollConnection.ts @@ -475,8 +475,6 @@ export class FirebaseIFrameScriptHolder { const iframeContents = '' + script + ''; try { this.myIFrame.doc.open(); - // TODO: 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) { @@ -719,10 +717,6 @@ export class FirebaseIFrameScriptHolder { const newScript = this.myIFrame.doc.createElement('script'); newScript.type = 'text/javascript'; newScript.async = true; - // TODO: 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/database/tsconfig.json b/packages/database/tsconfig.json index e2f493565c5..ce12ac3c5dc 100644 --- a/packages/database/tsconfig.json +++ b/packages/database/tsconfig.json @@ -3,15 +3,9 @@ "compilerOptions": { "outDir": "dist", "strict": false, - "downlevelIteration": true, - "plugins": [ - { - "name": "tsec", - "reportTsecDiagnosticsOnly": true, - } - ] + "downlevelIteration": true }, "exclude": [ "dist/**/*" ] -} +} \ No newline at end of file diff --git a/packages/messaging/package.json b/packages/messaging/package.json index 89aef86fc9d..1d4068ad73b 100644 --- a/packages/messaging/package.json +++ b/packages/messaging/package.json @@ -59,7 +59,6 @@ "@firebase/util": "1.9.7", "@firebase/component": "0.6.8", "idb": "7.1.1", - "safevalues": "0.6.0", "tslib": "^2.1.0" }, "devDependencies": { diff --git a/packages/messaging/src/helpers/registerDefaultSw.ts b/packages/messaging/src/helpers/registerDefaultSw.ts index b0be350bf72..239e6ed8244 100644 --- a/packages/messaging/src/helpers/registerDefaultSw.ts +++ b/packages/messaging/src/helpers/registerDefaultSw.ts @@ -24,7 +24,6 @@ export async function registerDefaultSw( messaging: MessagingService ): Promise { try { - // TODO: Use safevalues to register the service worker with a sanitized trustedResourceUrl. messaging.swRegistration = await navigator.serviceWorker.register( DEFAULT_SW_PATH, { diff --git a/yarn.lock b/yarn.lock index d3fb98832dc..49bbd54a546 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15435,11 +15435,6 @@ safe-stable-stringify@^1.1.0: resolved "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz" integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== -safevalues@0.6.0: - version "0.6.0" - resolved "https://registry.npmjs.org/safevalues/-/safevalues-0.6.0.tgz#425eadbdb699c13d8cfd932485983f858222362a" - integrity sha512-MZ7DcTOcIoPXN36/UONVE9BT0pmwlCr9WcS7Pj/q4FxOwr33FkWC0CUWj/THQXYWxf/F7urbhaHaOeFPSqGqHA== - sauce-connect-launcher@^1.2.7: version "1.3.2" resolved "https://registry.npmjs.org/sauce-connect-launcher/-/sauce-connect-launcher-1.3.2.tgz"