From 2d0a3e27f2b9e790fce2f2869a126460cdfd4cdc Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Mon, 4 Mar 2024 08:01:59 -0600 Subject: [PATCH] [PM-4566] Fix santization check issue found with webVaulUrl in notification bar (#7737) * [PM-673] Safari Notification Bar Does Not Show Folders * [PM-673] Refactoring Context Menu Implementations to Ensure Pages with No Logins Can Dismiss Notification Bar * [PM-673] Refactoring typing information for the LockedVaultPendingNotificationsItem typing * [PM-673] Refactoring typing information for notification background * [PM-673] Finishing out typing for potential extension messages to the notification bar; * [PM-673] Working through implementation details for the notification background * [PM-673] Fixing issues present with messaging re-implementation * [PM-673] Fixing issue with folders not populating within Safari notification bar * [PM-673] Fixing jest test issues present within implementation * [PM-673] Fixing issue present with webVaultUrl vulnerability * [PM-673] Fixing XSS Vulnerability within Notification Bar; * [PM-5670] Putting together a partial implementation for having messages appear on network error within the notification bar * [PM-673] Incorporating status update for when user has successfully saved credentials * [PM-673] Incorporating status update for when user has successfully saved credentials * [PM-5949] Refactor typing information for notification bar * [PM-5949] Fix jest tests for overlay background * [PM-5949] Removing unnused typing data * [PM-5949] Fixing lint error * [PM-5949] Adding jest tests for convertAddLoginQueueMessageToCipherView method * [PM-5949] Fixing jest test for overlay * [PM-5950] Fix Context Menu Update Race Condition and Refactor Implementation * [PM-5950] Adding jest test for cipherContextMenu.update method * [PM-5950] Adding documentation for method within MainContextMenuHandler * [PM-5950] Adding jest tests for the mainContextMenuHandler * [PM-673] Stripping unnecessary work for network drop issue * [PM-673] Stripping unnecessary work for network drop issue * [PM-2753] Prompt to Save New Login Credentials Silently Drops Data on Network Error * [PM-673] Stripping out work done for another ticket * [PM-4566] Fix santization check issue found with webVaulUrl in notification bar * [PM-4566] Refactoring implementation * [PM-5950] Removing unnecessary return value from MainContextMenuHandler.create method * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-2753] Implementing jest tests to validate logic changes * [PM-2753] Implementing jest tests to validate logic changes * [PM-2753] Implementing jest tests to validate logic changes * [PM-2753] Implementing jest tests to validate logic changes * [PM-2753] Incorporating addition of green and red borders when success or error events occur * [PM-4566] Adding jest test to validate changes within the notification background * [PM-5950] Fixing unawaited context menu promise * [PM-673] Merging changes in from main and fixing merge conflicts * [PM-2753] Merging work in from main and resolving merge conflicts * [PM-6122] Rework `window` call within NotificationBackground to function within content script * [PM-4566] Incorporating a more fleshed out implementation to remove queryParams from the notification bar url entirely * [PM-673] Fixing issue where updates to the added login were not triggering correctly * [PM-673] Merging changes in from main and fixing merge conflicts --- .../abstractions/notification.background.ts | 1 + .../notification.background.spec.ts | 18 ++++ .../background/notification.background.ts | 22 ++-- .../src/autofill/content/notification-bar.ts | 41 +++++-- .../abstractions/notification-bar.ts | 19 +++- apps/browser/src/autofill/notification/bar.ts | 102 ++++++++++-------- 6 files changed, 132 insertions(+), 71 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index ba6d18edbcb..94ab1a398d9 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -109,6 +109,7 @@ type NotificationBackgroundExtensionMessageHandlers = { bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise; checkNotificationQueue: ({ sender }: BackgroundSenderParam) => Promise; collectPageDetailsResponse: ({ message }: BackgroundMessageParam) => Promise; + getWebVaultUrlForNotification: () => string; }; export { diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index af92dae9ebc..29c8481e5f2 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -1332,5 +1332,23 @@ describe("NotificationBackground", () => { expect(openUnlockWindowSpy).toHaveBeenCalled(); }); }); + + describe("getWebVaultUrlForNotification", () => { + it("returns the web vault url", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "getWebVaultUrlForNotification", + }; + const webVaultUrl = "https://example.com"; + const environmentServiceSpy = jest + .spyOn(environmentService, "getWebVaultUrl") + .mockReturnValueOnce(webVaultUrl); + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(environmentServiceSpy).toHaveBeenCalled(); + expect(environmentServiceSpy).toHaveReturnedWith(webVaultUrl); + }); + }); }); }); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 42d8d47b23e..8ebf1bd26e3 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -6,7 +6,6 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { ThemeType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; @@ -58,6 +57,7 @@ export default class NotificationBackground { bgUnlockPopoutOpened: ({ message, sender }) => this.unlockVault(message, sender.tab), checkNotificationQueue: ({ sender }) => this.checkNotificationQueue(sender.tab), bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab), + getWebVaultUrlForNotification: () => this.getWebVaultUrl(), }; constructor( @@ -134,11 +134,9 @@ export default class NotificationBackground { notificationQueueMessage: NotificationQueueMessageItem, ) { const notificationType = notificationQueueMessage.type; - const webVaultURL = this.environmentService.getWebVaultUrl(); const typeData: Record = { isVaultLocked: notificationQueueMessage.wasVaultLocked, - theme: await this.getCurrentTheme(), - webVaultURL, + theme: await this.stateService.getTheme(), }; switch (notificationType) { @@ -158,18 +156,6 @@ export default class NotificationBackground { }); } - private async getCurrentTheme() { - const theme = await this.stateService.getTheme(); - - if (theme !== ThemeType.System) { - return theme; - } - - return window.matchMedia("(prefers-color-scheme: dark)").matches - ? ThemeType.Dark - : ThemeType.Light; - } - /** * Removes any login messages from the notification queue that * are associated with the specified tab. @@ -636,6 +622,10 @@ export default class NotificationBackground { return await firstValueFrom(this.folderService.folderViews$); } + private getWebVaultUrl(): string { + return this.environmentService.getWebVaultUrl(); + } + private async removeIndividualVault(): Promise { return await firstValueFrom( this.policyService.policyAppliesToActiveUser$(PolicyType.PersonalOwnership), diff --git a/apps/browser/src/autofill/content/notification-bar.ts b/apps/browser/src/autofill/content/notification-bar.ts index ac20401da58..5766017ebe8 100644 --- a/apps/browser/src/autofill/content/notification-bar.ts +++ b/apps/browser/src/autofill/content/notification-bar.ts @@ -4,6 +4,7 @@ import { } from "../background/abstractions/notification.background"; import AutofillField from "../models/autofill-field"; import { WatchedForm } from "../models/watched-form"; +import { NotificationBarIframeInitData } from "../notification/abstractions/notification-bar"; import { FormData } from "../services/abstractions/autofill.service"; import { GlobalSettings, UserSettings } from "../types"; import { getFromLocalStorage, setupExtensionDisconnectAction } from "../utils"; @@ -856,33 +857,36 @@ async function loadNotificationBar() { // Notification Bar Functions (open, close, height adjustment, etc.) function closeExistingAndOpenBar(type: string, typeData: any) { - const barQueryParams = { + const notificationBarInitData: NotificationBarIframeInitData = { type, isVaultLocked: typeData.isVaultLocked, theme: typeData.theme, removeIndividualVault: typeData.removeIndividualVault, - webVaultURL: typeData.webVaultURL, importType: typeData.importType, }; - const barQueryString = new URLSearchParams(barQueryParams).toString(); - const barPage = "notification/bar.html?" + barQueryString; + const notificationBarUrl = "notification/bar.html"; const frame = document.getElementById("bit-notification-bar-iframe") as HTMLIFrameElement; - if (frame != null && frame.src.indexOf(barPage) >= 0) { + if (frame != null && frame.src.indexOf(notificationBarUrl) >= 0) { return; } closeBar(false); - openBar(type, barPage); + openBar(type, notificationBarUrl, notificationBarInitData); } - function openBar(type: string, barPage: string) { + function openBar( + type: string, + barPage: string, + notificationBarInitData: NotificationBarIframeInitData, + ) { barType = type; if (document.body == null) { return; } + setupInitNotificationBarMessageListener(notificationBarInitData); const barPageUrl: string = chrome.runtime.getURL(barPage); notificationBarIframe = document.createElement("iframe"); @@ -901,7 +905,30 @@ async function loadNotificationBar() { document.body.appendChild(frameDiv); (notificationBarIframe.contentWindow.location as any) = barPageUrl; + } + function setupInitNotificationBarMessageListener(initData: NotificationBarIframeInitData) { + const handleInitNotificationBarMessage = (event: MessageEvent) => { + const { source, data } = event; + if ( + source !== notificationBarIframe.contentWindow || + data?.command !== "initNotificationBar" + ) { + return; + } + + notificationBarIframe.contentWindow.postMessage( + { command: "initNotificationBar", initData }, + "*", + ); + injectSpacer(); + window.removeEventListener("message", handleInitNotificationBarMessage); + }; + + window.addEventListener("message", handleInitNotificationBarMessage); + } + + function injectSpacer() { const spacer = document.createElement("div"); spacer.id = "bit-notification-bar-spacer"; spacer.style.cssText = "height: 42px;"; diff --git a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts index e2753893b1d..268617c419e 100644 --- a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts +++ b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts @@ -1,13 +1,26 @@ -import { SaveOrUpdateCipherResult } from "../../background/abstractions/notification.background"; +type NotificationBarIframeInitData = { + type?: string; + isVaultLocked?: boolean; + theme?: string; + removeIndividualVault?: boolean; + importType?: string; +}; type NotificationBarWindowMessage = { [key: string]: any; command: string; + error?: string; + initData?: NotificationBarIframeInitData; }; type NotificationBarWindowMessageHandlers = { [key: string]: CallableFunction; - saveCipherAttemptCompleted: ({ message }: { message: SaveOrUpdateCipherResult }) => void; + initNotificationBar: ({ message }: { message: NotificationBarWindowMessage }) => void; + saveCipherAttemptCompleted: ({ message }: { message: NotificationBarWindowMessage }) => void; }; -export { NotificationBarWindowMessage, NotificationBarWindowMessageHandlers }; +export { + NotificationBarIframeInitData, + NotificationBarWindowMessage, + NotificationBarWindowMessageHandlers, +}; diff --git a/apps/browser/src/autofill/notification/bar.ts b/apps/browser/src/autofill/notification/bar.ts index 1c21f891982..5d28bf83976 100644 --- a/apps/browser/src/autofill/notification/bar.ts +++ b/apps/browser/src/autofill/notification/bar.ts @@ -1,37 +1,42 @@ +import { ThemeType } from "@bitwarden/common/platform/enums"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import type { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { FilelessImportPort, FilelessImportType } from "../../tools/enums/fileless-import.enums"; -import { - AdjustNotificationBarMessageData, - SaveOrUpdateCipherResult, -} from "../background/abstractions/notification.background"; +import { AdjustNotificationBarMessageData } from "../background/abstractions/notification.background"; import { NotificationBarWindowMessageHandlers, NotificationBarWindowMessage, + NotificationBarIframeInitData, } from "./abstractions/notification-bar"; require("./bar.scss"); const logService = new ConsoleLogService(false); +let notificationBarIframeInitData: NotificationBarIframeInitData = {}; let windowMessageOrigin: string; const notificationBarWindowMessageHandlers: NotificationBarWindowMessageHandlers = { + initNotificationBar: ({ message }) => initNotificationBar(message), saveCipherAttemptCompleted: ({ message }) => handleSaveCipherAttemptCompletedMessage(message), }; -document.addEventListener("DOMContentLoaded", () => { - // delay 50ms so that we get proper body dimensions - setTimeout(load, 50); -}); - +globalThis.addEventListener("load", load); function load() { setupWindowMessageListener(); + postMessageToParent({ command: "initNotificationBar" }); +} - const theme = getQueryVariable("theme"); - document.documentElement.classList.add("theme_" + theme); +function initNotificationBar(message: NotificationBarWindowMessage) { + const { initData } = message; + if (!initData) { + return; + } + + notificationBarIframeInitData = initData; + const { isVaultLocked } = notificationBarIframeInitData; + setNotificationBarTheme(); - const isVaultLocked = getQueryVariable("isVaultLocked") == "true"; (document.getElementById("logo") as HTMLImageElement).src = isVaultLocked ? chrome.runtime.getURL("images/icon38_locked.png") : chrome.runtime.getURL("images/icon38.png"); @@ -55,16 +60,7 @@ function load() { startFilelessImport: chrome.i18n.getMessage("startFilelessImport"), }; - const logoLink = document.getElementById("logo-link") as HTMLAnchorElement; - logoLink.title = i18n.appName; - - // Update logo link to user's regional domain - const webVaultURL = getQueryVariable("webVaultURL"); - const newVaultURL = webVaultURL && decodeURIComponent(webVaultURL); - - if (newVaultURL && newVaultURL !== logoLink.href) { - logoLink.href = newVaultURL; - } + setupLogoLink(i18n); // i18n for "Add" template const addTemplate = document.getElementById("template-add") as HTMLTemplateElement; @@ -106,7 +102,7 @@ function load() { unlockTemplate.content.getElementById("unlock-text").textContent = i18n.notificationUnlockDesc; // i18n for "Fileless Import" (fileless-import) template - const isLpImport = getQueryVariable("importType") === FilelessImportType.LP; + const isLpImport = initData.importType === FilelessImportType.LP; const importTemplate = document.getElementById("template-fileless-import") as HTMLTemplateElement; const startImportButton = importTemplate.content.getElementById("start-fileless-import"); @@ -125,13 +121,14 @@ function load() { const closeButton = document.getElementById("close-button"); closeButton.title = i18n.close; - if (getQueryVariable("type") === "add") { + const notificationType = initData.type; + if (initData.type === "add") { handleTypeAdd(); - } else if (getQueryVariable("type") === "change") { + } else if (notificationType === "change") { handleTypeChange(); - } else if (getQueryVariable("type") === "unlock") { + } else if (notificationType === "unlock") { handleTypeUnlock(); - } else if (getQueryVariable("type") === "fileless-import") { + } else if (notificationType === "fileless-import") { handleTypeFilelessImport(); } @@ -146,20 +143,6 @@ function load() { adjustHeight(); } -function getQueryVariable(variable: string) { - const query = window.location.search.substring(1); - const vars = query.split("&"); - - for (let i = 0; i < vars.length; i++) { - const pair = vars[i].split("="); - if (pair[0] === variable) { - return pair[1]; - } - } - - return null; -} - function handleTypeAdd() { setContent(document.getElementById("template-add") as HTMLTemplateElement); @@ -219,7 +202,7 @@ function sendSaveCipherMessage(edit: boolean, folder?: string) { }); } -function handleSaveCipherAttemptCompletedMessage(message: SaveOrUpdateCipherResult) { +function handleSaveCipherAttemptCompletedMessage(message: NotificationBarWindowMessage) { const addSaveButtonContainers = document.querySelectorAll(".add-change-cipher-buttons"); const notificationBarOuterWrapper = document.getElementById("notification-bar-outer-wrapper"); if (message?.error) { @@ -233,7 +216,9 @@ function handleSaveCipherAttemptCompletedMessage(message: SaveOrUpdateCipherResu return; } const messageName = - getQueryVariable("type") === "add" ? "saveCipherAttemptSuccess" : "updateCipherAttemptSuccess"; + notificationBarIframeInitData.type === "add" + ? "saveCipherAttemptSuccess" + : "updateCipherAttemptSuccess"; addSaveButtonContainers.forEach((element) => { element.textContent = chrome.i18n.getMessage(messageName); @@ -261,7 +246,7 @@ function handleTypeUnlock() { * the Bitwarden vault. */ function handleTypeFilelessImport() { - const importType = getQueryVariable("importType"); + const importType = notificationBarIframeInitData.importType; const port = chrome.runtime.connect({ name: FilelessImportPort.NotificationBar }); setContent(document.getElementById("template-fileless-import") as HTMLTemplateElement); @@ -349,7 +334,7 @@ function getSelectedFolder(): string { } function removeIndividualVault(): boolean { - return getQueryVariable("removeIndividualVault") == "true"; + return notificationBarIframeInitData.removeIndividualVault; } function adjustHeight() { @@ -383,3 +368,30 @@ function handleWindowMessage(event: MessageEvent) { handler({ message }); } + +function setupLogoLink(i18n: Record) { + const logoLink = document.getElementById("logo-link") as HTMLAnchorElement; + logoLink.title = i18n.appName; + const setWebVaultUrlLink = (webVaultURL: string) => { + const newVaultURL = webVaultURL && decodeURIComponent(webVaultURL); + if (newVaultURL && newVaultURL !== logoLink.href) { + logoLink.href = newVaultURL; + } + }; + sendPlatformMessage({ command: "getWebVaultUrlForNotification" }, setWebVaultUrlLink); +} + +function setNotificationBarTheme() { + let theme = notificationBarIframeInitData.theme; + if (theme === ThemeType.System) { + theme = window.matchMedia("(prefers-color-scheme: dark)").matches + ? ThemeType.Dark + : ThemeType.Light; + } + + document.documentElement.classList.add(`theme_${theme}`); +} + +function postMessageToParent(message: NotificationBarWindowMessage) { + window.parent.postMessage(message, windowMessageOrigin || "*"); +}