From 0d85bdc9314f6a3365aae328079d1e4806492b17 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 15 Mar 2023 11:19:16 +1000 Subject: [PATCH] [PM-1397] Display a warning when a user attempts to auto-fill an iframe (#4994) * add settingsService.getEquivalentDomains * check that an iframe URL matches cipher.login.uris before autofilling * disable autofill on page load if it doesn't match * show a warning to the user on regular autofill if it doesn't match --------- Co-authored-by: Todd Martin <106564991+trmartin4@users.noreply.github.com> Co-authored-by: Robyn MacCallum --- .../autofill-service.factory.ts | 10 +- apps/browser/src/autofill/content/autofill.js | 14 +- .../autofill/models/autofill-page-details.ts | 1 - .../src/autofill/models/autofill-script.ts | 1 + .../services/abstractions/autofill.service.ts | 7 + .../src/autofill/services/autofill.service.ts | 132 +++++++++++++++++- .../browser/src/background/main.background.ts | 3 +- .../src/abstractions/settings.service.ts | 1 + libs/common/src/abstractions/state.service.ts | 2 +- libs/common/src/misc/utils.ts | 3 + libs/common/src/services/settings.service.ts | 21 +++ libs/common/src/services/state.service.ts | 2 +- .../src/vault/services/cipher.service.ts | 8 +- 13 files changed, 188 insertions(+), 17 deletions(-) diff --git a/apps/browser/src/autofill/background/service_factories/autofill-service.factory.ts b/apps/browser/src/autofill/background/service_factories/autofill-service.factory.ts index 87836198d7..a23b5e8dba 100644 --- a/apps/browser/src/autofill/background/service_factories/autofill-service.factory.ts +++ b/apps/browser/src/autofill/background/service_factories/autofill-service.factory.ts @@ -15,6 +15,10 @@ import { logServiceFactory, LogServiceInitOptions, } from "../../../background/service_factories/log-service.factory"; +import { + settingsServiceFactory, + SettingsServiceInitOptions, +} from "../../../background/service_factories/settings-service.factory"; import { stateServiceFactory, StateServiceInitOptions, @@ -33,7 +37,8 @@ export type AutoFillServiceInitOptions = AutoFillServiceOptions & StateServiceInitOptions & TotpServiceInitOptions & EventCollectionServiceInitOptions & - LogServiceInitOptions; + LogServiceInitOptions & + SettingsServiceInitOptions; export function autofillServiceFactory( cache: { autofillService?: AbstractAutoFillService } & CachedServices, @@ -49,7 +54,8 @@ export function autofillServiceFactory( await stateServiceFactory(cache, opts), await totpServiceFactory(cache, opts), await eventCollectionServiceFactory(cache, opts), - await logServiceFactory(cache, opts) + await logServiceFactory(cache, opts), + await settingsServiceFactory(cache, opts) ) ); } diff --git a/apps/browser/src/autofill/content/autofill.js b/apps/browser/src/autofill/content/autofill.js index f6fb7059ba..e79470b265 100644 --- a/apps/browser/src/autofill/content/autofill.js +++ b/apps/browser/src/autofill/content/autofill.js @@ -524,7 +524,6 @@ title: theDoc.title, url: theView.location.href, documentUrl: theDoc.location.href, - tabUrl: theView.location.href, forms: function (forms) { var formObj = {}; forms.forEach(function (f) { @@ -912,6 +911,19 @@ return; } + if (fillScript.untrustedIframe) { + // confirm() is blocked by sandboxed iframes, but we don't want to fill sandboxed iframes anyway. + // If this occurs, confirm() returns false without displaying the dialog box, and autofill will be aborted. + // The browser may print a message to the console, but this is not a standard error that we can handle. + var acceptedIframeWarning = confirm("The form is hosted by a different domain than the URI " + + "of your saved login. Choose OK to auto-fill anyway, or Cancel to stop. " + + "To prevent this warning in the future, save this URI, " + + window.location.hostname + ", to your login."); + if (!acceptedIframeWarning) { + return; + } + } + doOperation = function (ops, theOperation) { var op = ops[0]; if (void 0 === op) { diff --git a/apps/browser/src/autofill/models/autofill-page-details.ts b/apps/browser/src/autofill/models/autofill-page-details.ts index a1d683ba13..c45359a05a 100644 --- a/apps/browser/src/autofill/models/autofill-page-details.ts +++ b/apps/browser/src/autofill/models/autofill-page-details.ts @@ -12,7 +12,6 @@ export default class AutofillPageDetails { title: string; url: string; documentUrl: string; - tabUrl: string; /** * A collection of all of the forms in the page DOM, keyed by their `opid` */ diff --git a/apps/browser/src/autofill/models/autofill-script.ts b/apps/browser/src/autofill/models/autofill-script.ts index f18ac4ff69..c5988a1692 100644 --- a/apps/browser/src/autofill/models/autofill-script.ts +++ b/apps/browser/src/autofill/models/autofill-script.ts @@ -6,6 +6,7 @@ export default class AutofillScript { metadata: any = {}; autosubmit: any = null; savedUrls: string[]; + untrustedIframe: boolean; constructor(documentUUID: string) { this.documentUUID = documentUUID; diff --git a/apps/browser/src/autofill/services/abstractions/autofill.service.ts b/apps/browser/src/autofill/services/abstractions/autofill.service.ts index b65467eec7..2840c6dd5d 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill.service.ts @@ -1,3 +1,4 @@ +import { UriMatchType } from "@bitwarden/common/enums/uriMatchType"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import AutofillField from "../../models/autofill-field"; @@ -20,6 +21,7 @@ export interface AutoFillOptions { onlyVisibleFields?: boolean; fillNewPassword?: boolean; skipLastUsed?: boolean; + allowUntrustedIframe?: boolean; } export interface FormData { @@ -38,4 +40,9 @@ export abstract class AutofillService { fromCommand: boolean ) => Promise; doAutoFillActiveTab: (pageDetails: PageDetail[], fromCommand: boolean) => Promise; + iframeUrlMatches: ( + pageUrl: string, + loginItem: CipherView, + defaultUriMatch: UriMatchType + ) => boolean; } diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 188ca65509..35dbe4b8ea 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,14 +1,17 @@ import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { LogService } from "@bitwarden/common/abstractions/log.service"; +import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; import { TotpService } from "@bitwarden/common/abstractions/totp.service"; import { EventType } from "@bitwarden/common/enums/eventType"; import { FieldType } from "@bitwarden/common/enums/fieldType"; import { UriMatchType } from "@bitwarden/common/enums/uriMatchType"; +import { Utils } from "@bitwarden/common/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherType } from "@bitwarden/common/vault/enums/cipher-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; +import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view"; import { BrowserApi } from "../../browser/browserApi"; import { BrowserStateService } from "../../services/abstractions/browser-state.service"; @@ -34,6 +37,8 @@ export interface GenerateFillScriptOptions { onlyVisibleFields: boolean; fillNewPassword: boolean; cipher: CipherView; + tabUrl: string; + defaultUriMatch: UriMatchType; } export default class AutofillService implements AutofillServiceInterface { @@ -42,7 +47,8 @@ export default class AutofillService implements AutofillServiceInterface { private stateService: BrowserStateService, private totpService: TotpService, private eventCollectionService: EventCollectionService, - private logService: LogService + private logService: LogService, + private settingsService: SettingsService ) {} getFormsWithPasswordFields(pageDetails: AutofillPageDetails): FormData[] { @@ -84,7 +90,12 @@ export default class AutofillService implements AutofillServiceInterface { return formData; } - async doAutoFill(options: AutoFillOptions) { + /** + * Autofills a given tab with a given login item + * @param options Instructions about the autofill operation, including tab and login item + * @returns The TOTP code of the successfully autofilled login, if any + */ + async doAutoFill(options: AutoFillOptions): Promise { const tab = options.tab; if (!tab || !options.cipher || !options.pageDetails || !options.pageDetails.length) { throw new Error("Nothing to auto-fill."); @@ -93,6 +104,8 @@ export default class AutofillService implements AutofillServiceInterface { let totpPromise: Promise = null; const canAccessPremium = await this.stateService.getCanAccessPremium(); + const defaultUriMatch = (await this.stateService.getDefaultUriMatch()) ?? UriMatchType.Domain; + let didAutofill = false; options.pageDetails.forEach((pd) => { // make sure we're still on correct tab @@ -106,12 +119,23 @@ export default class AutofillService implements AutofillServiceInterface { onlyVisibleFields: options.onlyVisibleFields || false, fillNewPassword: options.fillNewPassword || false, cipher: options.cipher, + tabUrl: tab.url, + defaultUriMatch: defaultUriMatch, }); if (!fillScript || !fillScript.script || !fillScript.script.length) { return; } + if ( + fillScript.untrustedIframe && + options.allowUntrustedIframe != undefined && + !options.allowUntrustedIframe + ) { + this.logService.info("Auto-fill on page load was blocked due to an untrusted iframe."); + return; + } + // Add a small delay between operations fillScript.properties.delay_between_operations = 20; @@ -159,7 +183,18 @@ export default class AutofillService implements AutofillServiceInterface { } } - async doAutoFillOnTab(pageDetails: PageDetail[], tab: chrome.tabs.Tab, fromCommand: boolean) { + /** + * Autofills the specified tab with the next login item from the cache + * @param pageDetails The data scraped from the page + * @param tab The tab to be autofilled + * @param fromCommand Whether the autofill is triggered by a keyboard shortcut (`true`) or autofill on page load (`false`) + * @returns The TOTP code of the successfully autofilled login, if any + */ + async doAutoFillOnTab( + pageDetails: PageDetail[], + tab: chrome.tabs.Tab, + fromCommand: boolean + ): Promise { let cipher: CipherView; if (fromCommand) { cipher = await this.cipherService.getNextCipherForUrl(tab.url); @@ -188,6 +223,7 @@ export default class AutofillService implements AutofillServiceInterface { onlyEmptyFields: !fromCommand, onlyVisibleFields: !fromCommand, fillNewPassword: fromCommand, + allowUntrustedIframe: fromCommand, }); // Update last used index as autofill has succeed @@ -198,7 +234,13 @@ export default class AutofillService implements AutofillServiceInterface { return totpCode; } - async doAutoFillActiveTab(pageDetails: PageDetail[], fromCommand: boolean) { + /** + * Autofills the active tab with the next login item from the cache + * @param pageDetails The data scraped from the page + * @param fromCommand Whether the autofill is triggered by a keyboard shortcut (`true`) or autofill on page load (`false`) + * @returns The TOTP code of the successfully autofilled login, if any + */ + async doAutoFillActiveTab(pageDetails: PageDetail[], fromCommand: boolean): Promise { const tab = await this.getActiveTab(); if (!tab || !tab.url) { return; @@ -309,6 +351,10 @@ export default class AutofillService implements AutofillServiceInterface { fillScript.savedUrls = login?.uris?.filter((u) => u.match != UriMatchType.Never).map((u) => u.uri) ?? []; + const inIframe = pageDetails.url !== options.tabUrl; + fillScript.untrustedIframe = + inIframe && !this.iframeUrlMatches(pageDetails.url, options.cipher, options.defaultUriMatch); + if (!login.password || login.password === "") { // No password for this login. Maybe they just wanted to auto-fill some custom fields? fillScript = AutofillService.setFillScriptForFocus(filledFields, fillScript); @@ -742,6 +788,84 @@ export default class AutofillService implements AutofillServiceInterface { return fillScript; } + /** + * Determines whether to warn the user about filling an iframe + * @param pageUrl The url of the page/iframe, usually from AutofillPageDetails + * @param tabUrl The url of the tab, usually from the message sender (should not come from a content script because + * that is likely to be incorrect in the case of iframes) + * @param loginItem The cipher to be filled + * @returns `true` if the iframe is untrusted and the warning should be shown, `false` otherwise + */ + iframeUrlMatches(pageUrl: string, loginItem: CipherView, defaultUriMatch: UriMatchType): boolean { + // Check the pageUrl against cipher URIs using the configured match detection. + // If we are in this function at all, it is assumed that the tabUrl already matches a URL for `loginItem`, + // need to verify the pageUrl also matches one of the saved URIs using the match detection selected. + const uriMatched = loginItem.login.uris?.some((uri) => + this.uriMatches(uri, pageUrl, defaultUriMatch) + ); + + return uriMatched; + } + + // TODO should this be put in a common place (Utils maybe?) to be used both here and by CipherService? + private uriMatches(uri: LoginUriView, url: string, defaultUriMatch: UriMatchType): boolean { + const matchType = uri.match ?? defaultUriMatch; + + const matchDomains = [Utils.getDomain(url)]; + const equivalentDomains = this.settingsService.getEquivalentDomains(url); + if (equivalentDomains != null) { + matchDomains.push(...equivalentDomains); + } + + switch (matchType) { + case UriMatchType.Domain: + if (url != null && uri.domain != null && matchDomains.includes(uri.domain)) { + if (Utils.DomainMatchBlacklist.has(uri.domain)) { + const domainUrlHost = Utils.getHost(url); + if (!Utils.DomainMatchBlacklist.get(uri.domain).has(domainUrlHost)) { + return true; + } + } else { + return true; + } + } + break; + case UriMatchType.Host: { + const urlHost = Utils.getHost(url); + if (urlHost != null && urlHost === Utils.getHost(uri.uri)) { + return true; + } + break; + } + case UriMatchType.Exact: + if (url === uri.uri) { + return true; + } + break; + case UriMatchType.StartsWith: + if (url.startsWith(uri.uri)) { + return true; + } + break; + case UriMatchType.RegularExpression: + try { + const regex = new RegExp(uri.uri, "i"); + if (regex.test(url)) { + return true; + } + } catch (e) { + this.logService.error(e); + return false; + } + break; + case UriMatchType.Never: + default: + break; + } + + return false; + } + private fieldAttrsContain(field: AutofillField, containsVal: string) { if (!field) { return false; diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 8cb8744de2..b096ea5003 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -443,7 +443,8 @@ export default class MainBackground { this.stateService, this.totpService, this.eventCollectionService, - this.logService + this.logService, + this.settingsService ); this.containerService = new ContainerService(this.cryptoService, this.encryptService); this.auditService = new AuditService(this.cryptoFunctionService, this.apiService); diff --git a/libs/common/src/abstractions/settings.service.ts b/libs/common/src/abstractions/settings.service.ts index 1f6047dfa1..3283702279 100644 --- a/libs/common/src/abstractions/settings.service.ts +++ b/libs/common/src/abstractions/settings.service.ts @@ -6,5 +6,6 @@ export abstract class SettingsService { settings$: Observable; setEquivalentDomains: (equivalentDomains: string[][]) => Promise; + getEquivalentDomains: (url: string) => string[]; clear: (userId?: string) => Promise; } diff --git a/libs/common/src/abstractions/state.service.ts b/libs/common/src/abstractions/state.service.ts index 8df321092b..5c16578e06 100644 --- a/libs/common/src/abstractions/state.service.ts +++ b/libs/common/src/abstractions/state.service.ts @@ -245,7 +245,7 @@ export abstract class StateService { setEntityType: (value: string, options?: StorageOptions) => Promise; getEnvironmentUrls: (options?: StorageOptions) => Promise; setEnvironmentUrls: (value: EnvironmentUrls, options?: StorageOptions) => Promise; - getEquivalentDomains: (options?: StorageOptions) => Promise; + getEquivalentDomains: (options?: StorageOptions) => Promise; setEquivalentDomains: (value: string, options?: StorageOptions) => Promise; getEventCollection: (options?: StorageOptions) => Promise; setEventCollection: (value: EventData[], options?: StorageOptions) => Promise; diff --git a/libs/common/src/misc/utils.ts b/libs/common/src/misc/utils.ts index 02278e976b..fd7a5e337b 100644 --- a/libs/common/src/misc/utils.ts +++ b/libs/common/src/misc/utils.ts @@ -32,6 +32,9 @@ export class Utils { /(?:[\u231A\u231B\u23E9-\u23EC\u23F0\u23F3\u25FD\u25FE\u2614\u2615\u2648-\u2653\u267F\u2693\u26A1\u26AA\u26AB\u26BD\u26BE\u26C4\u26C5\u26CE\u26D4\u26EA\u26F2\u26F3\u26F5\u26FA\u26FD\u2705\u270A\u270B\u2728\u274C\u274E\u2753-\u2755\u2757\u2795-\u2797\u27B0\u27BF\u2B1B\u2B1C\u2B50\u2B55]|\uD83C[\uDC04\uDCCF\uDD8E\uDD91-\uDD9A\uDDE6-\uDDFF\uDE01\uDE1A\uDE2F\uDE32-\uDE36\uDE38-\uDE3A\uDE50\uDE51\uDF00-\uDF20\uDF2D-\uDF35\uDF37-\uDF7C\uDF7E-\uDF93\uDFA0-\uDFCA\uDFCF-\uDFD3\uDFE0-\uDFF0\uDFF4\uDFF8-\uDFFF]|\uD83D[\uDC00-\uDC3E\uDC40\uDC42-\uDCFC\uDCFF-\uDD3D\uDD4B-\uDD4E\uDD50-\uDD67\uDD7A\uDD95\uDD96\uDDA4\uDDFB-\uDE4F\uDE80-\uDEC5\uDECC\uDED0-\uDED2\uDED5-\uDED7\uDEEB\uDEEC\uDEF4-\uDEFC\uDFE0-\uDFEB]|\uD83E[\uDD0C-\uDD3A\uDD3C-\uDD45\uDD47-\uDD78\uDD7A-\uDDCB\uDDCD-\uDDFF\uDE70-\uDE74\uDE78-\uDE7A\uDE80-\uDE86\uDE90-\uDEA8\uDEB0-\uDEB6\uDEC0-\uDEC2\uDED0-\uDED6])/g; static readonly validHosts: string[] = ["localhost"]; static readonly minimumPasswordLength = 12; + static readonly DomainMatchBlacklist = new Map>([ + ["google.com", new Set(["script.google.com"])], + ]); static init() { if (Utils.inited) { diff --git a/libs/common/src/services/settings.service.ts b/libs/common/src/services/settings.service.ts index 4a6986e37f..b1ed84124a 100644 --- a/libs/common/src/services/settings.service.ts +++ b/libs/common/src/services/settings.service.ts @@ -40,6 +40,27 @@ export class SettingsService implements SettingsServiceAbstraction { await this.stateService.setSettings(settings); } + getEquivalentDomains(url: string): string[] { + const domain = Utils.getDomain(url); + if (domain == null) { + return null; + } + + const settings = this._settings.getValue(); + + let result: string[] = []; + + if (settings?.equivalentDomains != null) { + settings.equivalentDomains + .filter((ed) => ed.length > 0 && ed.includes(domain)) + .forEach((ed) => { + result = result.concat(ed); + }); + } + + return result; + } + async clear(userId?: string): Promise { if (userId == null || userId == (await this.stateService.getUserId())) { this._settings.next({}); diff --git a/libs/common/src/services/state.service.ts b/libs/common/src/services/state.service.ts index 9018bdc495..23ac052985 100644 --- a/libs/common/src/services/state.service.ts +++ b/libs/common/src/services/state.service.ts @@ -1585,7 +1585,7 @@ export class StateService< ); } - async getEquivalentDomains(options?: StorageOptions): Promise { + async getEquivalentDomains(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) )?.settings?.equivalentDomains; diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 25ce77f82e..60c8e59f97 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -49,10 +49,6 @@ import { CipherView } from "../models/view/cipher.view"; import { FieldView } from "../models/view/field.view"; import { PasswordHistoryView } from "../models/view/password-history.view"; -const DomainMatchBlacklist = new Map>([ - ["google.com", new Set(["script.google.com"])], -]); - export class CipherService implements CipherServiceAbstraction { private sortedCiphersCache: SortedCiphersCache = new SortedCiphersCache( this.sortCiphersByLastUsed @@ -456,9 +452,9 @@ export class CipherService implements CipherServiceAbstraction { switch (match) { case UriMatchType.Domain: if (domain != null && u.domain != null && matchingDomains.indexOf(u.domain) > -1) { - if (DomainMatchBlacklist.has(u.domain)) { + if (Utils.DomainMatchBlacklist.has(u.domain)) { const domainUrlHost = Utils.getHost(url); - if (!DomainMatchBlacklist.get(u.domain).has(domainUrlHost)) { + if (!Utils.DomainMatchBlacklist.get(u.domain).has(domainUrlHost)) { return true; } } else {