[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 <robyntmaccallum@gmail.com>
This commit is contained in:
Thomas Rittson 2023-03-15 11:19:16 +10:00 committed by GitHub
parent 6cbfdcf90a
commit 0d85bdc931
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 188 additions and 17 deletions

View File

@ -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)
)
);
}

View File

@ -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) {

View File

@ -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`
*/

View File

@ -6,6 +6,7 @@ export default class AutofillScript {
metadata: any = {};
autosubmit: any = null;
savedUrls: string[];
untrustedIframe: boolean;
constructor(documentUUID: string) {
this.documentUUID = documentUUID;

View File

@ -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<string>;
doAutoFillActiveTab: (pageDetails: PageDetail[], fromCommand: boolean) => Promise<string>;
iframeUrlMatches: (
pageUrl: string,
loginItem: CipherView,
defaultUriMatch: UriMatchType
) => boolean;
}

View File

@ -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<string> {
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<string> = 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<string> {
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<string> {
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;

View File

@ -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);

View File

@ -6,5 +6,6 @@ export abstract class SettingsService {
settings$: Observable<AccountSettingsSettings>;
setEquivalentDomains: (equivalentDomains: string[][]) => Promise<any>;
getEquivalentDomains: (url: string) => string[];
clear: (userId?: string) => Promise<void>;
}

View File

@ -245,7 +245,7 @@ export abstract class StateService<T extends Account = Account> {
setEntityType: (value: string, options?: StorageOptions) => Promise<void>;
getEnvironmentUrls: (options?: StorageOptions) => Promise<EnvironmentUrls>;
setEnvironmentUrls: (value: EnvironmentUrls, options?: StorageOptions) => Promise<void>;
getEquivalentDomains: (options?: StorageOptions) => Promise<any>;
getEquivalentDomains: (options?: StorageOptions) => Promise<string[][]>;
setEquivalentDomains: (value: string, options?: StorageOptions) => Promise<void>;
getEventCollection: (options?: StorageOptions) => Promise<EventData[]>;
setEventCollection: (value: EventData[], options?: StorageOptions) => Promise<void>;

View File

@ -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<string, Set<string>>([
["google.com", new Set(["script.google.com"])],
]);
static init() {
if (Utils.inited) {

View File

@ -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<void> {
if (userId == null || userId == (await this.stateService.getUserId())) {
this._settings.next({});

View File

@ -1585,7 +1585,7 @@ export class StateService<
);
}
async getEquivalentDomains(options?: StorageOptions): Promise<any> {
async getEquivalentDomains(options?: StorageOptions): Promise<string[][]> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
)?.settings?.equivalentDomains;

View File

@ -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<string, Set<string>>([
["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 {