1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-06-26 10:35:48 +02:00

[PM-5035] Fix autofill overlay clickjacking vulnerability that can be triggered by a malicious extension (#7001)

* [PM-5035] Fix autofill overlay clickjacking vulnerability that can be triggered by a malicious extension

* [PM-5035] Modifying method structure

* [PM-5035] Refactoring method structure

* [PM-5035] Refactoring method structure

* [PM-5035] Applying prettier to implementation
This commit is contained in:
Cesar Gonzalez 2023-12-11 15:44:15 -06:00 committed by GitHub
parent 33fd7094ca
commit 4d05b008f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 228 additions and 16 deletions

View File

@ -96,6 +96,7 @@ type OverlayButtonPortMessageHandlers = {
[key: string]: CallableFunction;
overlayButtonClicked: ({ port }: PortConnectionParam) => void;
closeAutofillOverlay: ({ port }: PortConnectionParam) => void;
forceCloseAutofillOverlay: ({ port }: PortConnectionParam) => void;
overlayPageBlurred: () => void;
redirectOverlayFocusOut: ({ message, port }: PortOnMessageHandlerParams) => void;
};
@ -103,6 +104,7 @@ type OverlayButtonPortMessageHandlers = {
type OverlayListPortMessageHandlers = {
[key: string]: CallableFunction;
checkAutofillOverlayButtonFocused: () => void;
forceCloseAutofillOverlay: ({ port }: PortConnectionParam) => void;
overlayPageBlurred: () => void;
unlockVault: ({ port }: PortConnectionParam) => void;
fillSelectedListItem: ({ message, port }: PortOnMessageHandlerParams) => void;

View File

@ -1056,13 +1056,29 @@ describe("OverlayBackground", () => {
describe("closeAutofillOverlay", () => {
it("sends a `closeOverlay` message to the sender tab", () => {
jest.spyOn(BrowserApi, "tabSendMessage");
jest.spyOn(BrowserApi, "tabSendMessageData");
sendPortMessage(buttonPortSpy, { command: "closeAutofillOverlay" });
expect(BrowserApi.tabSendMessage).toHaveBeenCalledWith(buttonPortSpy.sender.tab, {
command: "closeAutofillOverlay",
});
expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
buttonPortSpy.sender.tab,
"closeAutofillOverlay",
{ forceCloseOverlay: false },
);
});
});
describe("forceCloseAutofillOverlay", () => {
it("sends a `closeOverlay` message to the sender tab with a `forceCloseOverlay` flag of `true` set", () => {
jest.spyOn(BrowserApi, "tabSendMessageData");
sendPortMessage(buttonPortSpy, { command: "forceCloseAutofillOverlay" });
expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
buttonPortSpy.sender.tab,
"closeAutofillOverlay",
{ forceCloseOverlay: true },
);
});
});
@ -1113,6 +1129,20 @@ describe("OverlayBackground", () => {
});
});
describe("forceCloseAutofillOverlay", () => {
it("sends a `closeOverlay` message to the sender tab with a `forceCloseOverlay` flag of `true` set", () => {
jest.spyOn(BrowserApi, "tabSendMessageData");
sendPortMessage(listPortSpy, { command: "forceCloseAutofillOverlay" });
expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith(
listPortSpy.sender.tab,
"closeAutofillOverlay",
{ forceCloseOverlay: true },
);
});
});
describe("overlayPageBlurred", () => {
it("checks on the focus state of the overlay button", () => {
jest.spyOn(overlayBackground as any, "checkOverlayButtonFocused").mockImplementation();

View File

@ -68,11 +68,13 @@ class OverlayBackground implements OverlayBackgroundInterface {
private readonly overlayButtonPortMessageHandlers: OverlayButtonPortMessageHandlers = {
overlayButtonClicked: ({ port }) => this.handleOverlayButtonClicked(port),
closeAutofillOverlay: ({ port }) => this.closeOverlay(port),
forceCloseAutofillOverlay: ({ port }) => this.closeOverlay(port, true),
overlayPageBlurred: () => this.checkOverlayListFocused(),
redirectOverlayFocusOut: ({ message, port }) => this.redirectOverlayFocusOut(message, port),
};
private readonly overlayListPortMessageHandlers: OverlayListPortMessageHandlers = {
checkAutofillOverlayButtonFocused: () => this.checkOverlayButtonFocused(),
forceCloseAutofillOverlay: ({ port }) => this.closeOverlay(port, true),
overlayPageBlurred: () => this.checkOverlayButtonFocused(),
unlockVault: ({ port }) => this.unlockVault(port),
fillSelectedListItem: ({ message, port }) => this.fillSelectedOverlayListItem(message, port),
@ -268,9 +270,10 @@ class OverlayBackground implements OverlayBackgroundInterface {
* Sends a message to the sender tab to close the autofill overlay.
*
* @param sender - The sender of the port message
* @param forceCloseOverlay - Identifies whether the overlay should be force closed
*/
private closeOverlay({ sender }: chrome.runtime.Port) {
BrowserApi.tabSendMessage(sender.tab, { command: "closeAutofillOverlay" });
private closeOverlay({ sender }: chrome.runtime.Port, forceCloseOverlay = false) {
BrowserApi.tabSendMessageData(sender.tab, "closeAutofillOverlay", { forceCloseOverlay });
}
/**

View File

@ -16,6 +16,7 @@ type AutofillExtensionMessage = {
isOverlayCiphersPopulated?: boolean;
direction?: "previous" | "next";
isOpeningFullOverlay?: boolean;
forceCloseOverlay?: boolean;
};
};
@ -27,7 +28,7 @@ type AutofillExtensionMessageHandlers = {
collectPageDetailsImmediately: ({ message }: AutofillExtensionMessageParam) => void;
fillForm: ({ message }: AutofillExtensionMessageParam) => void;
openAutofillOverlay: ({ message }: AutofillExtensionMessageParam) => void;
closeAutofillOverlay: () => void;
closeAutofillOverlay: ({ message }: AutofillExtensionMessageParam) => void;
addNewVaultItemFromOverlay: () => void;
redirectOverlayFocusOut: ({ message }: AutofillExtensionMessageParam) => void;
updateIsOverlayCiphersPopulated: ({ message }: AutofillExtensionMessageParam) => void;

View File

@ -297,6 +297,30 @@ describe("AutofillInit", () => {
autofillInit["autofillOverlayContentService"].isCurrentlyFilling = false;
});
it("skips attempting to remove the overlay if the autofillOverlayContentService is not present", () => {
const newAutofillInit = new AutofillInit(undefined);
newAutofillInit.init();
jest.spyOn(newAutofillInit as any, "removeAutofillOverlay");
sendExtensionRuntimeMessage({
command: "closeAutofillOverlay",
data: { forceCloseOverlay: false },
});
expect(newAutofillInit["autofillOverlayContentService"]).toBe(undefined);
});
it("removes the autofill overlay if the message flags a forced closure", () => {
sendExtensionRuntimeMessage({
command: "closeAutofillOverlay",
data: { forceCloseOverlay: true },
});
expect(
autofillInit["autofillOverlayContentService"].removeAutofillOverlay,
).toHaveBeenCalled();
});
it("ignores the message if a field is currently focused", () => {
autofillInit["autofillOverlayContentService"].isFieldCurrentlyFocused = true;

View File

@ -20,7 +20,7 @@ class AutofillInit implements AutofillInitInterface {
collectPageDetailsImmediately: ({ message }) => this.collectPageDetails(message, true),
fillForm: ({ message }) => this.fillForm(message),
openAutofillOverlay: ({ message }) => this.openAutofillOverlay(message),
closeAutofillOverlay: () => this.removeAutofillOverlay(),
closeAutofillOverlay: ({ message }) => this.removeAutofillOverlay(message),
addNewVaultItemFromOverlay: () => this.addNewVaultItemFromOverlay(),
redirectOverlayFocusOut: ({ message }) => this.redirectOverlayFocusOut(message),
updateIsOverlayCiphersPopulated: ({ message }) => this.updateIsOverlayCiphersPopulated(message),
@ -153,7 +153,12 @@ class AutofillInit implements AutofillInitInterface {
* If the autofill is currently filling, only the overlay list will be
* removed.
*/
private removeAutofillOverlay() {
private removeAutofillOverlay(message?: AutofillExtensionMessage) {
if (message?.data?.forceCloseOverlay) {
this.autofillOverlayContentService?.removeAutofillOverlay();
return;
}
if (
!this.autofillOverlayContentService ||
this.autofillOverlayContentService.isFieldCurrentlyFocused

View File

@ -392,6 +392,23 @@ describe("AutofillOverlayIframeService", () => {
beforeEach(() => {
autofillOverlayIframeService.initOverlayIframe({ height: "0px" }, "title", "ariaAlert");
autofillOverlayIframeService["iframe"].dispatchEvent(new Event(EVENTS.LOAD));
portSpy = autofillOverlayIframeService["port"];
});
it("skips handling found mutations if excessive mutations are triggering", async () => {
jest.useFakeTimers();
jest
.spyOn(
autofillOverlayIframeService as any,
"isTriggeringExcessiveMutationObserverIterations",
)
.mockReturnValue(true);
jest.spyOn(autofillOverlayIframeService as any, "updateElementStyles");
autofillOverlayIframeService["iframe"].style.visibility = "hidden";
await flushPromises();
expect(autofillOverlayIframeService["updateElementStyles"]).not.toBeCalled();
});
it("reverts any styles changes made directly to the iframe", async () => {
@ -402,5 +419,47 @@ describe("AutofillOverlayIframeService", () => {
expect(autofillOverlayIframeService["iframe"].style.visibility).toBe("visible");
});
it("force closes the autofill overlay if more than 9 foreign mutations are triggered", async () => {
jest.useFakeTimers();
autofillOverlayIframeService["foreignMutationsCount"] = 10;
autofillOverlayIframeService["iframe"].src = "http://malicious-site.com";
await flushPromises();
expect(portSpy.postMessage).toBeCalledWith({ command: "forceCloseAutofillOverlay" });
});
it("force closes the autofill overlay if excessive mutations are being triggered", async () => {
jest.useFakeTimers();
autofillOverlayIframeService["mutationObserverIterations"] = 20;
autofillOverlayIframeService["iframe"].src = "http://malicious-site.com";
await flushPromises();
expect(portSpy.postMessage).toBeCalledWith({ command: "forceCloseAutofillOverlay" });
});
it("resets the excessive mutations and foreign mutation counters", async () => {
jest.useFakeTimers();
autofillOverlayIframeService["foreignMutationsCount"] = 9;
autofillOverlayIframeService["mutationObserverIterations"] = 19;
autofillOverlayIframeService["iframe"].src = "http://malicious-site.com";
jest.advanceTimersByTime(2001);
await flushPromises();
expect(autofillOverlayIframeService["foreignMutationsCount"]).toBe(0);
expect(autofillOverlayIframeService["mutationObserverIterations"]).toBe(0);
});
it("resets any mutated default attributes for the iframe", async () => {
jest.useFakeTimers();
autofillOverlayIframeService["iframe"].title = "some-other-title";
await flushPromises();
expect(autofillOverlayIframeService["iframe"].title).toBe("title");
});
});
});

View File

@ -30,6 +30,16 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf
colorScheme: "normal",
opacity: "0",
};
private defaultIframeAttributes: Record<string, string> = {
src: "",
title: "",
sandbox: "allow-scripts",
allowtransparency: "true",
tabIndex: "-1",
};
private foreignMutationsCount = 0;
private mutationObserverIterations = 0;
private mutationObserverIterationsResetTimeout: NodeJS.Timeout;
private readonly windowMessageHandlers: AutofillOverlayIframeWindowMessageHandlers = {
updateAutofillOverlayListHeight: (message) =>
this.updateElementStyles(this.iframe, message.styles),
@ -71,13 +81,14 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf
iframeTitle: string,
ariaAlert?: string,
) {
this.defaultIframeAttributes.src = chrome.runtime.getURL(this.iframePath);
this.defaultIframeAttributes.title = iframeTitle;
this.iframe = globalThis.document.createElement("iframe");
this.iframe.src = chrome.runtime.getURL(this.iframePath);
this.updateElementStyles(this.iframe, { ...this.iframeStyles, ...initStyles });
this.iframe.tabIndex = -1;
this.iframe.setAttribute("title", iframeTitle);
this.iframe.setAttribute("sandbox", "allow-scripts");
this.iframe.setAttribute("allowtransparency", "true");
for (const [attribute, value] of Object.entries(this.defaultIframeAttributes)) {
this.iframe.setAttribute(attribute, value);
}
this.iframe.addEventListener(EVENTS.LOAD, this.setupPortMessageListener);
if (ariaAlert) {
@ -290,9 +301,20 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf
* @param mutations - The mutations to the iframe element
*/
private handleMutations = (mutations: MutationRecord[]) => {
if (this.isTriggeringExcessiveMutationObserverIterations()) {
return;
}
for (let index = 0; index < mutations.length; index++) {
const mutation = mutations[index];
if (mutation.type !== "attributes" || mutation.attributeName !== "style") {
if (mutation.type !== "attributes") {
continue;
}
const element = mutation.target as HTMLElement;
if (mutation.attributeName !== "style") {
this.handleElementAttributeMutation(element);
continue;
}
@ -301,6 +323,41 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf
}
};
/**
* Handles mutations to the iframe element's attributes. This ensures that
* the iframe element's attributes are not modified by a third party source.
*
* @param element - The element to handle attribute mutations for
*/
private handleElementAttributeMutation(element: HTMLElement) {
const attributes = Array.from(element.attributes);
for (let attributeIndex = 0; attributeIndex < attributes.length; attributeIndex++) {
const attribute = attributes[attributeIndex];
if (attribute.name === "style") {
continue;
}
if (this.foreignMutationsCount >= 10) {
this.port?.postMessage({ command: "forceCloseAutofillOverlay" });
break;
}
const defaultIframeAttribute = this.defaultIframeAttributes[attribute.name];
if (!defaultIframeAttribute) {
this.iframe.removeAttribute(attribute.name);
this.foreignMutationsCount++;
continue;
}
if (attribute.value === defaultIframeAttribute) {
continue;
}
this.iframe.setAttribute(attribute.name, defaultIframeAttribute);
this.foreignMutationsCount++;
}
}
/**
* Observes the iframe element for mutations to its style attribute.
*/
@ -314,6 +371,35 @@ class AutofillOverlayIframeService implements AutofillOverlayIframeServiceInterf
private unobserveIframe() {
this.iframeMutationObserver.disconnect();
}
/**
* Identifies if the mutation observer is triggering excessive iterations.
* Will remove the autofill overlay if any set mutation observer is
* triggering excessive iterations.
*/
private isTriggeringExcessiveMutationObserverIterations() {
const resetCounters = () => {
this.mutationObserverIterations = 0;
this.foreignMutationsCount = 0;
};
if (this.mutationObserverIterationsResetTimeout) {
clearTimeout(this.mutationObserverIterationsResetTimeout);
}
this.mutationObserverIterations++;
this.mutationObserverIterationsResetTimeout = setTimeout(() => resetCounters(), 2000);
if (this.mutationObserverIterations > 20) {
clearTimeout(this.mutationObserverIterationsResetTimeout);
resetCounters();
this.port?.postMessage({ command: "forceCloseAutofillOverlay" });
return true;
}
return false;
}
}
export default AutofillOverlayIframeService;

View File

@ -118,7 +118,9 @@ class AutofillOverlayList extends AutofillOverlayPageElement {
private updateListItems(ciphers: OverlayCipherData[]) {
this.ciphers = ciphers;
this.currentCipherIndex = 0;
this.overlayListContainer.innerHTML = "";
if (this.overlayListContainer) {
this.overlayListContainer.innerHTML = "";
}
if (!ciphers?.length) {
this.buildNoResultsOverlayList();