Skip to content

Use safevalues to fix trusted types issues reported by tsec #8301

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 16, 2024
6 changes: 6 additions & 0 deletions .changeset/happy-trees-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/analytics': patch
'@firebase/app-check': patch
---

Begin using [safevalues](https://github.com/google/safevalues) to sanitize HTML vulnerable to XSS.
5 changes: 3 additions & 2 deletions packages/analytics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@
"@firebase/logger": "0.4.2",
"@firebase/util": "1.9.7",
"@firebase/component": "0.6.8",
"tslib": "^2.1.0"
"tslib": "^2.1.0",
"safevalues": "0.6.0"
},
"license": "Apache-2.0",
"devDependencies": {
"@firebase/app": "0.10.6",
"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"
},
Expand Down
78 changes: 5 additions & 73 deletions packages/analytics/src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,12 @@ import {
insertScriptTag,
wrapOrCreateGtag,
findGtagScriptOnPage,
promiseAllSettled,
createGtagTrustedTypesScriptURL,
createTrustedTypesPolicy
promiseAllSettled
} from './helpers';
import { GtagCommand, GTAG_URL } from './constants';
import { GtagCommand } 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';
Expand All @@ -50,71 +46,6 @@ 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();
Expand All @@ -136,8 +67,9 @@ describe('Gtag wrapping functions', () => {
insertScriptTag(customDataLayerName, fakeMeasurementId);
const scriptTag = findGtagScriptOnPage(customDataLayerName);
expect(scriptTag).to.not.be.null;
expect(scriptTag!.src).to.contain(`l=customDataLayerName`);
expect(scriptTag!.src).to.contain(`id=${fakeMeasurementId}`);
expect(scriptTag!.src).to.equal(
`https://www.googletagmanager.com/gtag/js?l=${customDataLayerName}&id=${fakeMeasurementId}`
);
});

// The test above essentially already touches this functionality but it is still valuable
Expand Down
60 changes: 10 additions & 50 deletions packages/analytics/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,12 @@ import {
import { DynamicConfig, DataLayer, Gtag, MinimalDynamicConfig } from './types';
import { GtagCommand, GTAG_URL } from './constants';
import { logger } from './logger';
import { AnalyticsError, ERROR_FACTORY } from './errors';
import { trustedResourceUrl } from 'safevalues';
import { safeScriptEl } from 'safevalues/dom';

// 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.
Expand All @@ -55,29 +42,6 @@ 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").
Expand All @@ -86,21 +50,17 @@ export function insertScriptTag(
dataLayerName: string,
measurementId: string
): void {
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.

const gtagScriptURL = `${GTAG_URL}?l=${dataLayerName}&id=${measurementId}`;
(script.src as string | TrustedScriptURL) = trustedTypesPolicy
? (trustedTypesPolicy as TrustedTypePolicy)?.createScriptURL(gtagScriptURL)
: gtagScriptURL;
//
// 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);

script.async = true;
document.head.appendChild(script);
Expand Down
1 change: 1 addition & 0 deletions packages/app-check/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"@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",
Expand Down
12 changes: 9 additions & 3 deletions packages/app-check/src/recaptcha.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import {
initializeV3,
initializeEnterprise,
getToken,
GreCAPTCHATopLevel
GreCAPTCHATopLevel,
RECAPTCHA_ENTERPRISE_URL,
RECAPTCHA_URL
} from './recaptcha';
import * as utils from './util';
import {
Expand Down Expand Up @@ -81,7 +83,9 @@ describe('recaptcha', () => {

expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
await initializeV3(app, FAKE_SITE_KEY);
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
const greCATPCHAScripts = findgreCAPTCHAScriptsOnPage();
expect(greCATPCHAScripts.length).to.equal(1);
expect(greCATPCHAScripts[0].src).to.equal(RECAPTCHA_URL);
});

it('creates invisible widget', async () => {
Expand Down Expand Up @@ -128,7 +132,9 @@ describe('recaptcha', () => {

expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
await initializeEnterprise(app, FAKE_SITE_KEY);
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
const greCAPTCHAScripts = findgreCAPTCHAScriptsOnPage();
expect(greCAPTCHAScripts.length).to.equal(1);
expect(greCAPTCHAScripts[0].src).to.equal(RECAPTCHA_ENTERPRISE_URL);
});

it('creates invisible widget', async () => {
Expand Down
15 changes: 13 additions & 2 deletions packages/app-check/src/recaptcha.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ 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';
Expand Down Expand Up @@ -166,14 +171,20 @@ function renderInvisibleWidget(

function loadReCAPTCHAV3Script(onload: () => void): void {
const script = document.createElement('script');
script.src = RECAPTCHA_URL;
safeScriptEl.setSrc(
script,
trustedResourceUrl`https://www.google.com/recaptcha/api.js`
);
script.onload = onload;
document.head.appendChild(script);
}

function loadReCAPTCHAEnterpriseScript(onload: () => void): void {
const script = document.createElement('script');
script.src = RECAPTCHA_ENTERPRISE_URL;
safeScriptEl.setSrc(
script,
trustedResourceUrl`https://www.google.com/recaptcha/enterprise.js`
);
script.onload = onload;
document.head.appendChild(script);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@
"@firebase/logger": "0.4.2",
"@firebase/util": "1.9.7",
"undici": "5.28.4",
"tslib": "^2.1.0"
"tslib": "^2.1.0",
"safevalues": "0.6.0"
},
"license": "Apache-2.0",
"devDependencies": {
Expand Down
4 changes: 4 additions & 0 deletions packages/auth/src/platform_browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ _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 => {
Expand Down
4 changes: 4 additions & 0 deletions packages/auth/src/platform_browser/load_js.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ 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 => {
Expand All @@ -65,6 +67,8 @@ 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'
Expand Down
10 changes: 8 additions & 2 deletions packages/auth/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
{
"extends": "../../config/tsconfig.base.json",
"compilerOptions": {
"outDir": "dist"
"outDir": "dist",
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
},
"exclude": [
"dist/**/*",
"demo/**/*"
]
}
}
10 changes: 8 additions & 2 deletions packages/database-compat/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
"compilerOptions": {
"outDir": "dist",
"strict": false,
"downlevelIteration": true
"downlevelIteration": true,
"plugins": [
{
"name": "tsec",
"reportTsecDiagnosticsOnly": true,
}
]
},
"exclude": [
"dist/**/*"
]
}
}
1 change: 1 addition & 0 deletions packages/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@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": {
Expand Down
6 changes: 6 additions & 0 deletions packages/database/src/realtime/BrowserPollConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ 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) {
Expand Down Expand Up @@ -717,6 +719,10 @@ 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 =
Expand Down
Loading
Loading