Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Revert introduction of safevalues #8395

Merged
merged 2 commits into from
Jul 26, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tender-apes-clap.md
Original file line number Diff line number Diff line change
@@ -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)
5 changes: 2 additions & 3 deletions packages/analytics/package.json
Original file line number Diff line number Diff line change
@@ -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"
},
78 changes: 73 additions & 5 deletions packages/analytics/src/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -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
60 changes: 50 additions & 10 deletions packages/analytics/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -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<T>(
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<TrustedTypePolicyOptions>
): Partial<TrustedTypePolicy> | undefined {
// Create a TrustedTypes policy that we can use for updating src
// properties
let trustedTypesPolicy: Partial<TrustedTypePolicy> | 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);
1 change: 0 additions & 1 deletion packages/app-check/package.json
Original file line number Diff line number Diff line change
@@ -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",
12 changes: 3 additions & 9 deletions packages/app-check/src/recaptcha.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
15 changes: 2 additions & 13 deletions packages/app-check/src/recaptcha.ts
Original file line number Diff line number Diff line change
@@ -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);
}
3 changes: 1 addition & 2 deletions packages/auth/package.json
Original file line number Diff line number Diff line change
@@ -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": {
4 changes: 0 additions & 4 deletions packages/auth/src/platform_browser/index.ts
Original file line number Diff line number Diff line change
@@ -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 => {
4 changes: 0 additions & 4 deletions packages/auth/src/platform_browser/load_js.test.ts
Original file line number Diff line number Diff line change
@@ -44,8 +44,6 @@ describe('platform-browser/load_js', () => {
loadJS(url: string): Promise<Event> {
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'
10 changes: 2 additions & 8 deletions packages/auth/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
{
"extends": "../../config/tsconfig.base.json",
"compilerOptions": {
"outDir": "dist",
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
"outDir": "dist"
},
"exclude": [
"dist/**/*",
"demo/**/*"
]
}
}
10 changes: 2 additions & 8 deletions packages/database-compat/tsconfig.json
Original file line number Diff line number Diff line change
@@ -3,15 +3,9 @@
"compilerOptions": {
"outDir": "dist",
"strict": false,
"downlevelIteration": true,
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
"downlevelIteration": true
},
"exclude": [
"dist/**/*"
]
}
}
1 change: 0 additions & 1 deletion packages/database/package.json
Original file line number Diff line number Diff line change
@@ -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": {
6 changes: 0 additions & 6 deletions packages/database/src/realtime/BrowserPollConnection.ts
Original file line number Diff line number Diff line change
@@ -475,8 +475,6 @@ export class FirebaseIFrameScriptHolder {
const iframeContents = '<html><body>' + script + '</body></html>';
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 =
Loading
Loading