diff --git a/README.md b/README.md index 96ee75f..dd58982 100644 --- a/README.md +++ b/README.md @@ -2175,14 +2175,17 @@ view would leave a wallet one click from deletion. code `4100` and opens no prompt. Two such origins are `null`, which the browser reports for a page with an opaque origin, such as one its server sandboxes with a Content-Security-Policy, and `file://`, which Chrome reports - for a local file. The background script prefers the toolbar popup - (`action.openPopup()`) and falls back to a separate popup window - (`src/background/index.js`, `requestApproval()`). Only one exists per site at - a time: a further connection request from a site whose prompt is still - unanswered is refused with EIP-1193 code `-32002` and opens no new prompt. If - that prompt was in a toolbar popup that closed before it connected, and the - toolbar popup has since been set to open something else, the refused request - shows that prompt again. + for a local file. Nor is such a page told the active address by + `accountsChanged`, even at a connected site's URL: the background names the + origin of the tab's URL on the event, and the content script passes the + address on only to a page whose own origin is that one. The background script + prefers the toolbar popup (`action.openPopup()`) and falls back to a separate + popup window (`src/background/index.js`, `requestApproval()`). Only one exists + per site at a time: a further connection request from a site whose prompt is + still unanswered is refused with EIP-1193 code `-32002` and opens no new + prompt. If that prompt was in a toolbar popup that closed before it connected, + and the toolbar popup has since been set to open something else, the refused + request shows that prompt again. - **Elements**: - "Connection Request" heading - Phishing warning banner (shown when the hostname is on the phishing diff --git a/src/background/index.js b/src/background/index.js index 33c9f83..09292de 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -1308,6 +1308,12 @@ async function broadcastAccountsChanged() { return; } for (const tab of tabs) { + // The tab's URL is all the background knows of the page in it, and a + // page at a site's URL may have another origin: a page its server + // sandboxes with a Content-Security-Policy has the origin "null". So + // the event names the origin it was judged by, and the content script + // passes the address on only to a page whose own origin is that one + // (src/content/index.js). const origin = tab.url ? new URL(tab.url).origin : ""; const hasPermission = activeAddress && @@ -1319,6 +1325,7 @@ async function broadcastAccountsChanged() { type: "AUTISTMASK_EVENT", eventName: "accountsChanged", data: hasPermission ? [activeAddress] : [], + origin, }).catch(() => {}); } } diff --git a/src/content/index.js b/src/content/index.js index cc1689f..bb532c5 100644 --- a/src/content/index.js +++ b/src/content/index.js @@ -49,14 +49,23 @@ window.addEventListener("message", (event) => { // Listen for events pushed from the background (e.g. accountsChanged) runtimeApi().onMessage.addListener((msg) => { - if (msg.type === "AUTISTMASK_EVENT") { - window.postMessage( - { - type: "AUTISTMASK_EVENT", - eventName: msg.eventName, - data: msg.data, - }, - "*", - ); + if (msg.type !== "AUTISTMASK_EVENT") return; + // The background chooses the addresses in accountsChanged for the site + // at the origin the event names, which is the origin of the tab's URL. + // This page hears them only when its own origin is that one. A page whose + // origin has no host is not a site: one its server sandboxes with a + // Content-Security-Policy has the origin "null" whatever its URL, and is + // told no address, as its requests are refused. + let data = msg.data; + if (msg.eventName === "accountsChanged" && msg.origin !== window.origin) { + data = []; } + window.postMessage( + { + type: "AUTISTMASK_EVENT", + eventName: msg.eventName, + data, + }, + "*", + ); }); diff --git a/tests/accountsChangedOpaqueOrigin.test.js b/tests/accountsChangedOpaqueOrigin.test.js new file mode 100644 index 0000000..dd0be4c --- /dev/null +++ b/tests/accountsChangedOpaqueOrigin.test.js @@ -0,0 +1,167 @@ +// A page whose origin has no host is never told an address by accountsChanged +// (https://git.eeqj.de/sneak/AutistMask/issues/564). +// +// When the user switches address, the background tells each open tab the new +// active address if the site at the tab's URL is connected. A page at a +// connected site's URL can still have another origin: one its server +// sandboxes with a Content-Security-Policy has the origin "null", and every +// request it makes is refused. Each test here switches the address in the real +// background, takes the event it sent to a tab at the site's URL, and hands +// that event to the real content script in a page of the given origin. + +const { networkById } = require("../src/shared/networks"); +const { makeStorageStub } = require("./support/storageStub"); + +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; +const SITE = "https://dapp.example"; + +const EXT_URL = "chrome-extension://autistmask/"; +const FROM_POPUP = { url: EXT_URL + "src/popup/index.html" }; + +// One address, with SITE remembered as allowed for it. +function storedProfile() { + const mainnet = networkById("mainnet"); + return { + hasWallet: true, + wallets: [ + { + name: "Wallet 1", + type: "hd", + addresses: [ + { address: ADDRESS, balance: "0", tokenBalances: [] }, + ], + }, + ], + activeAddress: ADDRESS, + networkId: "mainnet", + rpcUrl: mainnet.defaultRpcUrl, + blockscoutUrl: mainnet.defaultBlockscoutUrl, + allowedSites: { [ADDRESS]: [SITE] }, + deniedSites: {}, + trackedTokens: [], + }; +} + +async function settle() { + for (let i = 0; i < 50; i++) await Promise.resolve(); +} + +afterEach(() => { + delete global.chrome; + delete global.window; + jest.resetModules(); +}); + +// The popup tells the background the active address changed, and the one +// open tab, at SITE's URL, is sent an event. That event is returned. +async function eventSentToSiteTab() { + jest.resetModules(); + jest.doMock("../src/shared/balances", () => ({ + getProvider: () => ({}), + refreshBalances: jest.fn(async () => {}), + })); + jest.doMock("../src/shared/phishingDomains", () => ({ + isPhishingDomain: () => false, + })); + jest.doMock("../src/shared/alarms", () => ({ + BALANCE_REFRESH_ALARM: "balance", + BALANCE_REFRESH_PERIOD_MINUTES: 1, + ensureRecurringAlarms: jest.fn(async () => {}), + registerAlarmHandlers: jest.fn(), + })); + + let messageListener = null; + const sentToTabs = []; + global.chrome = { + storage: makeStorageStub({ autistmask: storedProfile() }), + runtime: { + getURL: (path) => EXT_URL + path, + onMessage: { + addListener: (fn) => { + messageListener = fn; + }, + }, + onConnect: { addListener: () => {} }, + getPlatformInfo: (cb) => cb({ os: "linux" }), + lastError: null, + }, + windows: { + remove: (id, cb) => { + if (cb) cb(); + }, + onRemoved: { addListener: () => {} }, + }, + tabs: { + query: (q, cb) => cb([{ id: 1, url: SITE + "/app" }]), + sendMessage: (tabId, msg, cb) => { + sentToTabs.push(msg); + if (cb) cb(); + }, + }, + action: { setPopup: () => {} }, + }; + + require("../src/background/index"); + messageListener( + { type: "AUTISTMASK_ACTIVE_CHANGED" }, + FROM_POPUP, + () => {}, + ); + await settle(); + + expect(sentToTabs).toHaveLength(1); + return sentToTabs[0]; +} + +// The content script in a page whose origin is `pageOrigin` is handed `event` +// from the background. What it posts to the page is returned. +function postedToPage(event, pageOrigin) { + jest.resetModules(); + let eventListener = null; + const posted = []; + global.chrome = { + runtime: { + onMessage: { + addListener: (fn) => { + eventListener = fn; + }, + }, + }, + }; + global.window = { + origin: pageOrigin, + addEventListener: () => {}, + postMessage: (data) => posted.push(data), + }; + + require("../src/content/index"); + eventListener(event); + + return posted; +} + +describe("accountsChanged after an address switch, at a connected site's URL", () => { + test("the site's own page is told the active address", async () => { + const event = await eventSentToSiteTab(); + + expect(postedToPage(event, SITE)).toEqual([ + { + type: "AUTISTMASK_EVENT", + eventName: "accountsChanged", + data: [ADDRESS], + }, + ]); + }); + + test("a page whose origin has no host is told no address", async () => { + const event = await eventSentToSiteTab(); + + expect(postedToPage(event, "null")).toEqual([ + { + type: "AUTISTMASK_EVENT", + eventName: "accountsChanged", + data: [], + }, + ]); + }); +});