fix: a page whose origin has no host is never told an address by accountsChanged (closes #564)
The background judges each tab by its URL's origin, the only one it can see, so a page its server sandboxes, whose own origin is null, was still sent the new active address on an address switch although all its requests are refused. The background now names the origin it judged by on each accountsChanged event, and the content script passes the address to the page only when the page's own window.origin is that origin, an empty list otherwise. An event naming no origin also reaches the page as an empty list. chainChanged is unchanged. The README's SiteApproval section says the same. Judgement call: covered by a unit test that sets window.origin; no browser-suite test. Model: opus-5-5
This commit was merged in pull request #570.
This commit is contained in:
@@ -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
|
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
|
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
|
sandboxes with a Content-Security-Policy, and `file://`, which Chrome reports
|
||||||
for a local file. The background script prefers the toolbar popup
|
for a local file. Nor is such a page told the active address by
|
||||||
(`action.openPopup()`) and falls back to a separate popup window
|
`accountsChanged`, even at a connected site's URL: the background names the
|
||||||
(`src/background/index.js`, `requestApproval()`). Only one exists per site at
|
origin of the tab's URL on the event, and the content script passes the
|
||||||
a time: a further connection request from a site whose prompt is still
|
address on only to a page whose own origin is that one. The background script
|
||||||
unanswered is refused with EIP-1193 code `-32002` and opens no new prompt. If
|
prefers the toolbar popup (`action.openPopup()`) and falls back to a separate
|
||||||
that prompt was in a toolbar popup that closed before it connected, and the
|
popup window (`src/background/index.js`, `requestApproval()`). Only one exists
|
||||||
toolbar popup has since been set to open something else, the refused request
|
per site at a time: a further connection request from a site whose prompt is
|
||||||
shows that prompt again.
|
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**:
|
- **Elements**:
|
||||||
- "Connection Request" heading
|
- "Connection Request" heading
|
||||||
- Phishing warning banner (shown when the hostname is on the phishing
|
- Phishing warning banner (shown when the hostname is on the phishing
|
||||||
|
|||||||
@@ -1308,6 +1308,12 @@ async function broadcastAccountsChanged() {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
for (const tab of tabs) {
|
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 origin = tab.url ? new URL(tab.url).origin : "";
|
||||||
const hasPermission =
|
const hasPermission =
|
||||||
activeAddress &&
|
activeAddress &&
|
||||||
@@ -1319,6 +1325,7 @@ async function broadcastAccountsChanged() {
|
|||||||
type: "AUTISTMASK_EVENT",
|
type: "AUTISTMASK_EVENT",
|
||||||
eventName: "accountsChanged",
|
eventName: "accountsChanged",
|
||||||
data: hasPermission ? [activeAddress] : [],
|
data: hasPermission ? [activeAddress] : [],
|
||||||
|
origin,
|
||||||
}).catch(() => {});
|
}).catch(() => {});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+18
-9
@@ -49,14 +49,23 @@ window.addEventListener("message", (event) => {
|
|||||||
|
|
||||||
// Listen for events pushed from the background (e.g. accountsChanged)
|
// Listen for events pushed from the background (e.g. accountsChanged)
|
||||||
runtimeApi().onMessage.addListener((msg) => {
|
runtimeApi().onMessage.addListener((msg) => {
|
||||||
if (msg.type === "AUTISTMASK_EVENT") {
|
if (msg.type !== "AUTISTMASK_EVENT") return;
|
||||||
window.postMessage(
|
// 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.
|
||||||
type: "AUTISTMASK_EVENT",
|
// This page hears them only when its own origin is that one. A page whose
|
||||||
eventName: msg.eventName,
|
// origin has no host is not a site: one its server sandboxes with a
|
||||||
data: msg.data,
|
// 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,
|
||||||
|
},
|
||||||
|
"*",
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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: [],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user