From fe349870ab9b67012d3c01419642ee59223609af Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 15:03:16 +0000 Subject: [PATCH] harden: take a request's origin from the frame that sent it (closes #407) Where the browser gives no sender.origin (Firefox before 126), the background credited a page's request to the tab's page, so a frame from another site counted as the site embedding it, and with no tab it used an origin the page wrote into the message. It now uses the origin of sender.url, the frame that sent the message, and refuses the request with code 4100 when the browser gives neither. The content script no longer writes an origin into the message. Model: opus-5-5 --- TODO.md | 9 +++ src/background/index.js | 27 +++++--- src/content/index.js | 1 - tests/rpcOrigin.test.js | 147 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 tests/rpcOrigin.test.js diff --git a/TODO.md b/TODO.md index b68b5a0..78cc1a4 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: A page's request is credited only to the site the browser says + sent it ([#407](https://git.eeqj.de/sneak/AutistMask/issues/407)). Where the + browser does not give the sender's origin (Firefox before 126), the background + used the tab's page, so a frame from another site would have been treated as + the site embedding it, and with no tab it used an origin the page wrote into + the message. It now uses the URL of the frame that sent the message, and + refuses the request with code 4100 when the browser gives neither. The content + script no longer writes an origin into the message. + - 2026-10-04: `make test` takes 8-13s on the shared build host, down from 17-25s, measured in alternating runs before and after the change ([#428](https://git.eeqj.de/sneak/AutistMask/issues/428)). Each popup boot in diff --git a/src/background/index.js b/src/background/index.js index 5172cba..649b527 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -1288,18 +1288,29 @@ if (windowsNs && windowsNs.onRemoved) { // Listen for messages from content scripts and popup runtime.onMessage.addListener((msg, sender, sendResponse) => { if (msg.type === "AUTISTMASK_RPC") { - // Derive origin from trusted sender info to prevent origin spoofing. - // Chrome MV3 provides sender.origin; Firefox MV2 fallback uses sender.tab.url. - let trustedOrigin = msg.origin; // fallback only if sender info unavailable - if (sender.origin) { - trustedOrigin = sender.origin; - } else if (sender.tab && sender.tab.url) { + // The origin is the one the browser reports for the sender, never one + // the message carries. Firefox before 126 gives no sender.origin, so + // the origin of sender.url is used: the frame that sent the message, + // not the tab's page, which may be another site embedding that + // frame. With neither, the request is refused. + let trustedOrigin = sender.origin; + if (!trustedOrigin && sender.url) { try { - trustedOrigin = new URL(sender.tab.url).origin; + trustedOrigin = new URL(sender.url).origin; } catch { - // keep fallback + // an unparseable URL leaves the origin unknown } } + if (!trustedOrigin) { + sendResponse({ + error: { + code: 4100, + message: + "The wallet could not tell which site sent this request.", + }, + }); + return false; + } handleRpc(msg.method, msg.params, trustedOrigin) .then((response) => { sendResponse(response); diff --git a/src/content/index.js b/src/content/index.js index 4c71f72..cc1689f 100644 --- a/src/content/index.js +++ b/src/content/index.js @@ -30,7 +30,6 @@ window.addEventListener("message", (event) => { id, method, params, - origin: location.origin, }) .then((response) => { if (response) { diff --git a/tests/rpcOrigin.test.js b/tests/rpcOrigin.test.js new file mode 100644 index 0000000..cecfdab --- /dev/null +++ b/tests/rpcOrigin.test.js @@ -0,0 +1,147 @@ +// Which site a page's request is attributed to. +// +// The background takes a request's origin from what the browser says sent the +// message: sender.origin, or on Firefox before 126, which has no +// sender.origin, the origin of sender.url — the frame that sent it. It used to +// fall back to the tab's page and then to an origin the message itself +// carried, so a request from a frame was credited to the site embedding it, +// and a request the browser said nothing about was credited to whatever the +// page wrote (https://git.eeqj.de/sneak/AutistMask/issues/407). +// +// Every sender here lacks sender.origin, as on old Firefox. The connection +// check on eth_accounts is what shows which site a request was credited to. + +const { makeStorageStub } = require("./support/storageStub"); + +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +// The site the persisted state has connected, and one it has never heard of. +const CONNECTED_ORIGIN = "https://dapp.example"; +const CONNECTED_HOSTNAME = "dapp.example"; +const STRANGER_ORIGIN = "https://stranger.example"; + +async function settle() { + for (let i = 0; i < 50; i++) await Promise.resolve(); +} + +afterEach(() => { + delete global.chrome; +}); + +function loadBackground() { + 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(), + })); + + const storage = makeStorageStub({ + autistmask: { + networkId: "mainnet", + wallets: [ + { + name: "Wallet 1", + type: "hd", + addresses: [ + { address: ADDRESS, balance: "0", tokenBalances: [] }, + ], + }, + ], + activeAddress: ADDRESS, + allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] }, + deniedSites: {}, + }, + }); + + let messageListener = null; + global.chrome = { + storage, + runtime: { + getURL: (path) => "chrome-extension://autistmask/" + path, + onMessage: { + addListener: (fn) => { + messageListener = fn; + }, + }, + onConnect: { addListener: () => {} }, + lastError: null, + }, + windows: { onRemoved: { addListener: () => {} } }, + action: { setPopup: () => {} }, + }; + + require("../src/background/index"); + + // Ask for eth_accounts. `claimedOrigin` is an origin written into the + // message, as the content script used to send. + return async function accounts(sender, claimedOrigin) { + let result = null; + messageListener( + { + type: "AUTISTMASK_RPC", + method: "eth_accounts", + params: [], + origin: claimedOrigin, + }, + sender, + (r) => { + result = r; + }, + ); + await settle(); + return result; + }; +} + +describe("a request is attributed to the frame that sent it", () => { + test("a stranger's frame on a connected site gets no address", async () => { + const accounts = loadBackground(); + + const result = await accounts({ + url: STRANGER_ORIGIN + "/frame.html", + tab: { url: CONNECTED_ORIGIN + "/" }, + }); + + expect(result).toEqual({ result: [] }); + }); + + test("a connected site's frame on a stranger's page gets the address", async () => { + const accounts = loadBackground(); + + const result = await accounts({ + url: CONNECTED_ORIGIN + "/frame.html", + tab: { url: STRANGER_ORIGIN + "/" }, + }); + + expect(result).toEqual({ result: [ADDRESS] }); + }); +}); + +describe("a request the browser does not say the sender of", () => { + test("is refused, whatever the tab or the message says", async () => { + const accounts = loadBackground(); + + const result = await accounts( + { tab: { url: CONNECTED_ORIGIN + "/" } }, + CONNECTED_ORIGIN, + ); + + expect(result).toEqual({ + error: { + code: 4100, + message: + "The wallet could not tell which site sent this request.", + }, + }); + }); +});