harden: key remembered site permissions by full origin (closes #402)
check / check (push) Failing after 2s
e2e / e2e-chrome (push) Failing after 3s
e2e / e2e-firefox (push) Failing after 2s

allowedSites and deniedSites held the bare hostname, so a grant to
https://dapp.example also authorised http://dapp.example and every port
on that host, and the connection, transaction and signature prompts
named only the hostname. Both lists now store and match the full origin
(scheme://host[:port]), the key the connections approved without
Remember already used. The prompts, the Settings site lists and
AUTISTMASK_REMOVE_SITE use the origin too. Entries saved by hostname
are not migrated (pre-1.0): they match no site.

Model: opus-5-5
This commit is contained in:
2026-10-04 15:43:58 +00:00
parent f24b5bca19
commit e557f21bb0
29 changed files with 470 additions and 231 deletions
+140
View File
@@ -0,0 +1,140 @@
// The connection, transaction and signature prompts name the site by its full
// origin, scheme and port included, not by its bare hostname
// (https://git.eeqj.de/sneak/AutistMask/issues/402). A page served over http,
// or on another port, of a host the user trusts over https must not raise a
// prompt that reads as that trusted site.
//
// Driven against a minimal DOM stub in the shape
// tests/contractCreation.test.js uses.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { state } = require("../src/shared/state");
const approval = require("../src/popup/views/approval");
// The site asking, in cleartext and on a port, which is what the hostname
// alone, dapp.example, used to hide.
const ORIGIN = "http://dapp.example:8080";
const FROM = "0x0000000000000000000000000000000000000a11";
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
function makeElement(id) {
const classes = new Set();
const el = {
id,
textContent: "",
value: "",
innerHTML: "",
disabled: false,
style: {},
dataset: {},
classList: {
add: (...names) => names.forEach((n) => classes.add(n)),
remove: (...names) => names.forEach((n) => classes.delete(n)),
contains: (n) => classes.has(n),
toggle: (n, force) => {
const on = force === undefined ? !classes.has(n) : force;
if (on) classes.add(n);
else classes.delete(n);
return on;
},
},
addEventListener: () => {},
querySelectorAll: () => [],
appendChild: () => {},
};
// Views reach for .parentElement to hide whole sections.
Object.defineProperty(el, "parentElement", {
get: () => node(id + "-parent"),
});
return el;
}
function makeDocument() {
const els = new Map();
return {
getElementById(id) {
// The debug banner is created on demand by helpers.js; absent
// is the state a non-debug, non-testnet popup is in.
if (id === "debug-banner") return null;
if (!els.has(id)) els.set(id, makeElement(id));
return els.get(id);
},
createElement: () => makeElement("created"),
body: { prepend: () => {} },
};
}
function node(id) {
return globalThis.document.getElementById(id);
}
// Open the prompt the background describes with `details`, the way the popup
// does: it asks for the approval and show() draws it.
async function openApproval(details) {
globalThis.document = makeDocument();
globalThis.window = { location: { search: "" } };
globalThis.chrome.runtime = {
connect: () => ({ postMessage: () => {} }),
sendMessage: (msg, reply) => {
if (!reply) return;
if (msg.type !== "AUTISTMASK_GET_APPROVAL") return reply(null);
reply({
origin: ORIGIN,
isPhishingDomain: false,
approvedFrom: FROM,
...details,
});
},
};
approval.init({});
await approval.show(1);
}
beforeEach(() => {
state.wallets = [];
state.activeAddress = FROM;
state.viewData = {};
state.viewStack = [];
state.currentView = null;
});
test("the connection prompt shows the origin", async () => {
await openApproval({});
expect(node("approve-origin").textContent).toBe(ORIGIN);
});
test("the transaction prompt shows the origin", async () => {
await openApproval({
type: "tx",
approvedTx: {
type: 2,
from: FROM,
chainId: "0x1",
nonce: "0x7",
gasLimit: "0x5208",
maxPriorityFeePerGas: "0x3b9aca00",
maxFeePerGas: "0x77359400",
to: RECIPIENT,
value: "0x0",
data: "0x",
accessList: [],
},
});
expect(node("approve-tx-origin").textContent).toBe(ORIGIN);
});
test("the signature prompt shows the origin", async () => {
await openApproval({
type: "sign",
// "Hello" as the hex a dApp passes to personal_sign.
signParams: {
method: "personal_sign",
message: "0x48656c6c6f",
from: FROM,
},
});
expect(node("approve-sign-origin").textContent).toBe(ORIGIN);
});
+85 -17
View File
@@ -39,7 +39,6 @@ const other = new Wallet(OTHER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example";
// A page the wallet has never been connected to, whose requests are refused.
const UNCONNECTED_ORIGIN = "https://stranger.example";
const EXT_URL = "chrome-extension://autistmask/";
@@ -199,7 +198,7 @@ function loadBackground(options) {
networkId: "mainnet",
rpcUrl: "https://rpc.invalid",
activeAddress: signer.address,
allowedSites: { [signer.address]: [HOSTNAME] },
allowedSites: { [signer.address]: [ORIGIN] },
deniedSites: {},
};
@@ -2193,12 +2192,54 @@ describe("removing an address ends a site's connection to it", () => {
});
});
// A remembered permission belongs to the origin it was granted to, scheme and
// port included (https://git.eeqj.de/sneak/AutistMask/issues/402). The stored
// state allows ORIGIN, https://dapp.example. A cleartext page on the same host,
// which a network attacker can serve, and another port on it are other sites.
describe("a remembered permission is held by the full origin", () => {
for (const origin of ["http://dapp.example", "https://dapp.example:8443"]) {
test(`an https grant does not authorise ${origin}`, async () => {
const bg = loadBackground();
expect(await siteAccounts(bg, ORIGIN)).toEqual({
result: [signer.address],
});
expect(await siteAccounts(bg, origin)).toEqual({ result: [] });
const send = bg.requestTx(TX_PARAMS, origin);
await settle();
expect(send.result()).toEqual({
error: { code: 4100, message: "Unauthorized" },
});
expect(send.id()).toBeNull();
});
}
test("Remember stores the origin, so the cleartext page on that host is asked again", async () => {
const bg = loadBackground({ actionPopup: true });
const granted = bg.requestSite(FRESH_ORIGIN);
await settle();
const grantedId = granted.id();
bg.connectApproval(grantedId).decide(true, true);
await settle();
expect(granted.result()).toEqual({ result: [signer.address] });
expect(
bg.storage.read("autistmask").allowedSites[signer.address],
).toEqual([ORIGIN, FRESH_ORIGIN]);
const cleartext = bg.requestSite("http://fresh.example");
await settle();
// Unanswered: it is waiting on a prompt of its own.
expect(cleartext.result()).toBeNull();
expect(cleartext.id()).not.toBe(grantedId);
});
});
// Settings lists the sites allowed without "Remember", which only the
// background holds, and removing a site there, from either list, disconnects
// it. These drive the real Settings view against the real background and
// click the [x] the user clicks.
describe("removing a site in Settings disconnects it", () => {
// FRESH_ORIGIN on another port, so its hostname is FRESH_ORIGIN's.
// The host of FRESH_ORIGIN on another port, which makes it another site.
const FRESH_OTHER_PORT = "https://fresh.example:8443";
// A site list's container. Its [x] buttons, data attributes and all, are
@@ -2226,9 +2267,9 @@ describe("removing a site in Settings disconnects it", () => {
return list;
}
// The hostnames a site list shows.
// The origins a site list shows.
function listed(list) {
return [...list.innerHTML.matchAll(/data-hostname="([^"]*)"/g)].map(
return [...list.innerHTML.matchAll(/data-origin="([^"]*)"/g)].map(
(match) => match[1],
);
}
@@ -2259,10 +2300,10 @@ describe("removing a site in Settings disconnects it", () => {
allowed: element("settings-allowed-sites"),
connected: element("settings-connected-sites"),
// Click the [x] beside a site, and let what it sends run.
remove: async (list, hostname) => {
expect(listed(list)).toContain(hostname);
remove: async (list, origin) => {
expect(listed(list)).toContain(origin);
const button = list.buttons.find(
(b) => b.dataset.hostname === hostname,
(b) => b.dataset.origin === origin,
);
await button.click();
await settle();
@@ -2274,14 +2315,15 @@ describe("removing a site in Settings disconnects it", () => {
delete global.document;
});
test("Settings lists a site connected without Remember", async () => {
test("Settings lists each site by its origin", async () => {
const bg = loadBackground({ actionPopup: true });
await connect(bg, FRESH_ORIGIN, false);
await connect(bg, FRESH_OTHER_PORT, true);
const settings = await openSettings(bg);
expect(listed(settings.connected)).toEqual(["fresh.example"]);
expect(listed(settings.allowed)).toEqual([HOSTNAME]);
expect(listed(settings.connected)).toEqual([FRESH_ORIGIN]);
expect(listed(settings.allowed)).toEqual([ORIGIN, FRESH_OTHER_PORT]);
});
test("removing a site connected without Remember disconnects it and tells its tabs", async () => {
@@ -2293,6 +2335,7 @@ describe("removing a site in Settings disconnects it", () => {
cb([
{ id: 1, url: FRESH_ORIGIN + "/app" },
{ id: 2, url: ORIGIN + "/app" },
{ id: 3, url: FRESH_OTHER_PORT + "/app" },
]),
sendMessage: (tabId, msg, cb) => {
sentToTabs.push({ tabId, msg });
@@ -2301,7 +2344,7 @@ describe("removing a site in Settings disconnects it", () => {
};
const settings = await openSettings(bg);
await settings.remove(settings.connected, "fresh.example");
await settings.remove(settings.connected, FRESH_ORIGIN);
expect(await siteAccounts(bg)).toEqual({ result: [] });
expect(sentToTabs).toEqual([
@@ -2321,20 +2364,45 @@ describe("removing a site in Settings disconnects it", () => {
});
});
// The same hostname can hold both kinds of connection: one origin allowed
// without Remember, then another, on a different port, allowed with it.
// One origin can hold both kinds of connection under two addresses:
// remembered for one, allowed without Remember for the other.
test("removing a remembered site also ends its connection made without Remember", async () => {
const bg = loadBackground({ actionPopup: true });
const stored = bg.storage.read("autistmask");
stored.wallets[0].addresses.push({
address: other.address,
balance: "0",
tokenBalances: [],
});
bg.storage.write("autistmask", stored);
await connect(bg, FRESH_ORIGIN, true);
bg.setActiveAddress(other.address);
const pending = bg.requestSite(FRESH_ORIGIN);
await settle();
bg.connectApproval(pending.id()).decide(true, false);
await settle();
expect(pending.result()).toEqual({ result: [other.address] });
const settings = await openSettings(bg);
await settings.remove(settings.allowed, FRESH_ORIGIN);
expect(await siteAccounts(bg, FRESH_ORIGIN)).toEqual({ result: [] });
});
test("removing a remembered site leaves the same host on another port connected", async () => {
const bg = loadBackground({ actionPopup: true });
await connect(bg, FRESH_ORIGIN, false);
await connect(bg, FRESH_OTHER_PORT, true);
const settings = await openSettings(bg);
await settings.remove(settings.allowed, "fresh.example");
await settings.remove(settings.allowed, FRESH_OTHER_PORT);
expect(await siteAccounts(bg, FRESH_OTHER_PORT)).toEqual({
result: [],
});
expect(await siteAccounts(bg, FRESH_ORIGIN)).toEqual({ result: [] });
expect(await siteAccounts(bg, FRESH_ORIGIN)).toEqual({
result: [signer.address],
});
});
test("a page can neither remove a site nor list the connected ones", async () => {
@@ -2343,7 +2411,7 @@ describe("removing a site in Settings disconnects it", () => {
const page = { url: FRESH_ORIGIN + "/index.html" };
const remove = bg.send(
{ type: "AUTISTMASK_REMOVE_SITE", hostname: "fresh.example" },
{ type: "AUTISTMASK_REMOVE_SITE", origin: FRESH_ORIGIN },
page,
);
const list = bg.send({ type: "AUTISTMASK_GET_CONNECTED_SITES" }, page);
+1 -2
View File
@@ -33,7 +33,6 @@ const signer = new Wallet(SIGNER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const CONNECTED_ORIGIN = "https://dapp.example";
const CONNECTED_HOSTNAME = "dapp.example";
const EXT_URL = "chrome-extension://autistmask/";
const MAINNET = networkById("mainnet");
@@ -81,7 +80,7 @@ function storedProfile(networkId) {
networkId,
rpcUrl: net.defaultRpcUrl,
blockscoutUrl: net.defaultBlockscoutUrl,
allowedSites: { [signer.address]: [CONNECTED_HOSTNAME] },
allowedSites: { [signer.address]: [CONNECTED_ORIGIN] },
deniedSites: {},
trackedTokens: [],
lastBalanceRefresh: 0,
+1 -2
View File
@@ -19,7 +19,6 @@ 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";
const MAINNET = networkById("mainnet");
@@ -86,7 +85,7 @@ function loadBackground() {
tokenHolderCache: {},
fraudContracts: [],
activeAddress: ADDRESS,
allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
allowedSites: { [ADDRESS]: [CONNECTED_ORIGIN] },
deniedSites: {},
};
const storage = makeStorageStub({ autistmask: persisted });
+1 -2
View File
@@ -17,7 +17,6 @@ const { networkById } = require("../src/shared/networks");
const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const CONNECTED_ORIGIN = "https://dapp.example";
const CONNECTED_HOSTNAME = "dapp.example";
const UNKNOWN_ORIGIN = "https://stranger.example";
const MAINNET = networkById("mainnet");
@@ -41,7 +40,7 @@ function storedProfile(networkId) {
networkId,
rpcUrl: networkById(networkId).defaultRpcUrl,
blockscoutUrl: networkById(networkId).defaultBlockscoutUrl,
allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
allowedSites: { [ADDRESS]: [CONNECTED_ORIGIN] },
deniedSites: {},
trackedTokens: [],
};
+2 -3
View File
@@ -21,7 +21,6 @@ const { makeStorageStub } = require("./support/storageStub");
const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const CONNECTED_ORIGIN = "https://dapp.example";
const CONNECTED_HOSTNAME = "dapp.example";
const MAINNET = networkById("mainnet");
const SEPOLIA = networkById("sepolia");
@@ -50,7 +49,7 @@ function storedProfile(networkId) {
networkId,
rpcUrl: CUSTOM_RPC,
blockscoutUrl: CUSTOM_BLOCKSCOUT,
allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
allowedSites: { [ADDRESS]: [CONNECTED_ORIGIN] },
deniedSites: {},
trackedTokens: [{ address: TOKEN, symbol: "DAI", decimals: 18 }],
theme: "dark",
@@ -168,7 +167,7 @@ describe("a chain switch on a worker that never loaded state", () => {
expect(after.wallets).toEqual(walletFixture());
expect(after.hasWallet).toBe(true);
expect(after.activeAddress).toBe(ADDRESS);
expect(after.allowedSites).toEqual({ [ADDRESS]: [CONNECTED_HOSTNAME] });
expect(after.allowedSites).toEqual({ [ADDRESS]: [CONNECTED_ORIGIN] });
expect(after.trackedTokens).toEqual([
{ address: TOKEN, symbol: "DAI", decimals: 18 },
]);
+1 -2
View File
@@ -29,7 +29,6 @@ const signer = new Wallet(SIGNER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const CONNECTED_ORIGIN = "https://dapp.example";
const CONNECTED_HOSTNAME = "dapp.example";
const EXT_URL = "chrome-extension://autistmask/";
const SEPOLIA = networkById("sepolia");
@@ -67,7 +66,7 @@ function storedProfile(networkId) {
networkId,
rpcUrl: net.defaultRpcUrl,
blockscoutUrl: net.defaultBlockscoutUrl,
allowedSites: { [signer.address]: [CONNECTED_HOSTNAME] },
allowedSites: { [signer.address]: [CONNECTED_ORIGIN] },
deniedSites: {},
trackedTokens: [],
};
+1 -1
View File
@@ -150,7 +150,7 @@ async function openTxApproval(to, data) {
if (msg.type !== "AUTISTMASK_GET_APPROVAL") return reply(null);
reply({
type: "tx",
hostname: "dapp.example",
origin: "https://dapp.example",
isPhishingDomain: false,
approvedFrom: FROM,
approvedTx: {
+11 -5
View File
@@ -140,8 +140,14 @@ function load() {
state.selectedWallet = 0;
state.selectedAddress = 0;
state.activeAddress = A0;
state.allowedSites = { [A0]: ["a.example"], [B0]: ["b.example"] };
state.deniedSites = { [B0]: ["c.example"], [C0]: ["d.example"] };
state.allowedSites = {
[A0]: ["https://a.example"],
[B0]: ["https://b.example"],
};
state.deniedSites = {
[B0]: ["https://c.example"],
[C0]: ["https://d.example"],
};
state.viewStack = ["main", "settings"];
state.currentView = "settings";
@@ -388,8 +394,8 @@ describe("deleting without the password", () => {
await click("btn-delete-wallet-lost-confirm");
const saved = (await storage.get("autistmask")).autistmask;
expect(saved.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["d.example"] });
expect(saved.allowedSites).toEqual({ [A0]: ["https://a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["https://d.example"] });
});
// The route shares finishDelete() with the password route, so the
@@ -437,7 +443,7 @@ describe("deleting without the password", () => {
test("deleting the last wallet lands on Welcome with nothing left", async () => {
const { deleteWallet, state, storage } = load();
state.wallets = [wallet("Wallet 1", "secret-one", [A0])];
state.allowedSites = { [A0]: ["a.example"] };
state.allowedSites = { [A0]: ["https://a.example"] };
state.deniedSites = {};
await openLostPassword(deleteWallet, 0);
+9 -10
View File
@@ -438,11 +438,10 @@ step(
await d.switchToWindow(popup);
await d.waitVisible("#view-approve-site");
const hostname = await d.text("#approve-hostname");
const origin = await d.text("#approve-origin");
assert(
hostname === "127.0.0.1",
"the site prompt names the wrong origin: " +
JSON.stringify(hostname),
origin === env.server.origin,
"the site prompt names the wrong origin: " + JSON.stringify(origin),
);
const shown = await d.text("#approve-address");
assert(
@@ -494,16 +493,16 @@ step(
const screen = await d.execute(
`return {
hostname: document.getElementById("approve-sign-hostname").textContent,
origin: document.getElementById("approve-sign-origin").textContent,
type: document.getElementById("approve-sign-type").textContent,
message: document.getElementById("approve-sign-message").textContent,
from: document.getElementById("approve-sign-from").textContent,
};`,
);
assert(
screen.hostname === "127.0.0.1",
screen.origin === env.server.origin,
"the sign prompt names the wrong origin: " +
JSON.stringify(screen.hostname),
JSON.stringify(screen.origin),
);
assert(
screen.type === "Personal message",
@@ -571,7 +570,7 @@ step(
const screen = await d.execute(
`return {
hostname: document.getElementById("approve-tx-hostname").textContent,
origin: document.getElementById("approve-tx-origin").textContent,
from: document.getElementById("approve-tx-from").textContent,
to: document.getElementById("approve-tx-to").textContent,
value: document.getElementById("approve-tx-value").textContent,
@@ -582,9 +581,9 @@ step(
};`,
);
assert(
screen.hostname === "127.0.0.1",
screen.origin === env.server.origin,
"the transaction prompt names the wrong origin: " +
JSON.stringify(screen.hostname),
JSON.stringify(screen.origin),
);
assert(
screen.from.toLowerCase().includes(env.address.toLowerCase()),
+16 -19
View File
@@ -35,6 +35,7 @@ const {
const {
DAPP_ORIGIN,
DAPP_URL,
PHISHING_DAPP_ORIGIN,
PHISHING_DAPP_URL,
FEE_ESTIMATE_WEI,
FEE_RESERVE_WEI,
@@ -2471,8 +2472,6 @@ test("a token whose symbol() returns markup renders as text (#307)", async (env)
// dApp, with real funds, against a real network. The RPC is stubbed
// throughout. That pass stays on the human list before 1.0.0.
const DAPP_HOSTNAME = new URL(DAPP_URL).hostname;
// The personal_sign payload. Sent as hex, which is what dApps send and what
// the popup requires — it calls getBytes() on the message — and displayed on
// the approval screen as the decoded text, which is what the user is agreeing
@@ -3016,11 +3015,10 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as
try {
await visible(popup, "#view-approve-site");
const hostname = await popup.locator("#approve-hostname").innerText();
const origin = await popup.locator("#approve-origin").innerText();
assert(
hostname === DAPP_HOSTNAME,
"the site prompt names the wrong origin: " +
JSON.stringify(hostname),
origin === DAPP_ORIGIN,
"the site prompt names the wrong origin: " + JSON.stringify(origin),
);
// The control for the phishing test below: this origin is not on the
@@ -3101,7 +3099,6 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
// check and the real approval screen. Nothing about the list is stubbed —
// there is nothing left to stub, since the extension no longer fetches it.
const phishingDapp = await openDapp(env.ctx, PHISHING_DAPP_URL);
const hostname = new URL(PHISHING_DAPP_URL).hostname;
try {
await reserveApprovalTab(env);
await startRequest(
@@ -3114,15 +3111,15 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
try {
await visible(popup, "#view-approve-site");
const shown = await popup.locator("#approve-hostname").innerText();
const shown = await popup.locator("#approve-origin").innerText();
assert(
shown === hostname,
shown === PHISHING_DAPP_ORIGIN,
"the site prompt names the wrong origin: " +
JSON.stringify(shown),
);
await visible(popup, "#approve-site-phishing-warning");
console.log("# phishing warning shown for " + hostname);
console.log("# phishing warning shown for " + PHISHING_DAPP_ORIGIN);
// Not remembered: a remembered decision for this origin would
// outlive the test.
@@ -3152,15 +3149,15 @@ test("personal_sign signs, and the signature recovers to the address (#183)", as
const boundary = await watchApprovalBoundary(popup, env);
const screen = await popup.evaluate(() => ({
hostname: document.getElementById("approve-sign-hostname").textContent,
origin: document.getElementById("approve-sign-origin").textContent,
type: document.getElementById("approve-sign-type").textContent,
message: document.getElementById("approve-sign-message").textContent,
from: document.getElementById("approve-sign-from").textContent,
}));
assert(
screen.hostname === DAPP_HOSTNAME,
screen.origin === DAPP_ORIGIN,
"the sign prompt names the wrong origin: " +
JSON.stringify(screen.hostname),
JSON.stringify(screen.origin),
);
assert(
screen.type === "Personal message",
@@ -3245,15 +3242,15 @@ test("eth_signTypedData_v4 signs, and the signature recovers (#183)", async (env
const boundary = await watchApprovalBoundary(popup, env);
const screen = await popup.evaluate(() => ({
hostname: document.getElementById("approve-sign-hostname").textContent,
origin: document.getElementById("approve-sign-origin").textContent,
type: document.getElementById("approve-sign-type").textContent,
message: document.getElementById("approve-sign-message").innerText,
from: document.getElementById("approve-sign-from").textContent,
}));
assert(
screen.hostname === DAPP_HOSTNAME,
screen.origin === DAPP_ORIGIN,
"the typed data prompt names the wrong origin: " +
JSON.stringify(screen.hostname),
JSON.stringify(screen.origin),
);
assert(
screen.type === "Typed data (EIP-712)",
@@ -3351,7 +3348,7 @@ test("eth_sendTransaction signs the approved transaction and broadcasts it (#183
const boundary = await watchApprovalBoundary(popup, env);
const screen = await popup.evaluate(() => ({
hostname: document.getElementById("approve-tx-hostname").textContent,
origin: document.getElementById("approve-tx-origin").textContent,
from: document.getElementById("approve-tx-from").textContent,
to: document.getElementById("approve-tx-to").textContent,
value: document.getElementById("approve-tx-value").textContent,
@@ -3361,9 +3358,9 @@ test("eth_sendTransaction signs the approved transaction and broadcasts it (#183
.classList.contains("hidden"),
}));
assert(
screen.hostname === DAPP_HOSTNAME,
screen.origin === DAPP_ORIGIN,
"the transaction prompt names the wrong origin: " +
JSON.stringify(screen.hostname),
JSON.stringify(screen.origin),
);
assert(
screen.from.toLowerCase().includes(env.expectedAddress.toLowerCase()),
+23 -12
View File
@@ -59,24 +59,32 @@ describe("the floor under allowedSites and deniedSites", () => {
}
});
test(`an ${field} entry whose value is not a hostname list is dropped`, () => {
for (const bad of ["dapp.example", 42, null, { a: 1 }, true]) {
test(`an ${field} entry whose value is not an origin list is dropped`, () => {
for (const bad of [
"https://dapp.example",
42,
null,
{ a: 1 },
true,
]) {
expect(
normalizePersisted({ [field]: { [ADDRESS]: bad } })[field],
).toEqual({});
}
});
test(`a hostname that is not text is dropped from an ${field} entry`, () => {
test(`an origin that is not text is dropped from an ${field} entry`, () => {
expect(
normalizePersisted({
[field]: { [ADDRESS]: [42, null, "dapp.example", {}] },
[field]: {
[ADDRESS]: [42, null, "https://dapp.example", {}],
},
})[field],
).toEqual({ [ADDRESS]: ["dapp.example"] });
).toEqual({ [ADDRESS]: ["https://dapp.example"] });
});
test(`a real ${field} map survives, copied not shared`, () => {
const saved = { [field]: { [ADDRESS]: ["dapp.example"] } };
const saved = { [field]: { [ADDRESS]: ["https://dapp.example"] } };
const out = normalizePersisted(saved);
@@ -87,10 +95,13 @@ describe("the floor under allowedSites and deniedSites", () => {
test(`a good ${field} entry beside a malformed one survives`, () => {
const out = normalizePersisted({
[field]: { [ADDRESS]: ["dapp.example"], [TOKEN_ADDRESS]: 42 },
[field]: {
[ADDRESS]: ["https://dapp.example"],
[TOKEN_ADDRESS]: 42,
},
});
expect(out[field]).toEqual({ [ADDRESS]: ["dapp.example"] });
expect(out[field]).toEqual({ [ADDRESS]: ["https://dapp.example"] });
});
test(`a stored own "__proto__" key in ${field} is dropped`, () => {
@@ -100,7 +111,7 @@ describe("the floor under allowedSites and deniedSites", () => {
// saveState()'s merge hands to the prototype setter on the next
// write.
const saved = JSON.parse(
'{"' + field + '":{"__proto__":["evil.invalid"]}}',
'{"' + field + '":{"__proto__":["https://evil.invalid"]}}',
);
const out = normalizePersisted(saved);
@@ -197,7 +208,7 @@ describe("a malformed allowedSites entry", () => {
const MALFORMED = [
{ name: "a string", value: "notalist" },
{ name: "a number", value: 42 },
{ name: "a record", value: { hostnames: ["dapp.example"] } },
{ name: "a record", value: { origins: ["https://dapp.example"] } },
];
for (const { name, value } of MALFORMED) {
@@ -231,7 +242,7 @@ describe("a malformed allowedSites entry", () => {
const env = await bootPopup(
unversionedValidProfile({
allowedSites: {
[ADDRESS]: ["dapp.example"],
[ADDRESS]: ["https://dapp.example"],
[TOKEN_ADDRESS]: "notalist",
},
}),
@@ -239,7 +250,7 @@ describe("a malformed allowedSites entry", () => {
expect(env.pageErrors).toEqual([]);
expect(env.storage.read("autistmask").allowedSites).toEqual({
[ADDRESS]: ["dapp.example"],
[ADDRESS]: ["https://dapp.example"],
});
});
+2 -2
View File
@@ -165,7 +165,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["evil.invalid"]}'),
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
],
holds: siteMapHolds,
},
@@ -177,7 +177,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["evil.invalid"]}'),
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
],
holds: siteMapHolds,
},
+1 -2
View File
@@ -17,7 +17,6 @@ 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() {
@@ -58,7 +57,7 @@ function loadBackground() {
},
],
activeAddress: ADDRESS,
allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
allowedSites: { [ADDRESS]: [CONNECTED_ORIGIN] },
deniedSites: {},
},
});
+1 -1
View File
@@ -232,7 +232,7 @@ async function confirmSend(amount, token = "ETH") {
async function approveTxWithFeePerGas(maxFeePerGas) {
approvalDetails = {
type: "tx",
hostname: "dapp.example",
origin: "https://dapp.example",
approvedFrom: HOLDER,
approvedTx: {
to: RECIPIENT,
+33 -31
View File
@@ -270,31 +270,32 @@ describe("background refresh racing a wallet deleted on another page", () => {
});
});
// allowedSites/deniedSites: { [address]: [hostname, ...] }. Mutated in place
// from two different contexts — src/background/index.js:592-599 pushes a
// newly approved hostname onto state.allowedSites[activeAddress], and the
// Settings "revoke" button (src/popup/views/settings.js:55-68) filters a
// hostname out of state[key][addr] in place, deleting the address key
// entirely once its list is empty — the exact membership-vs-whole-field
// pattern that made the whole-field `wallets` diff unsafe, on a
// security-relevant field: a stale whole-field save here can resurrect a
// revoked permission or wipe a freshly granted one.
// allowedSites/deniedSites: { [address]: [origin, ...] }. Mutated in place
// from two different contexts — rememberSiteChoice() in
// src/background/index.js pushes a newly approved origin onto
// state.allowedSites[activeAddress], and the Settings "revoke" button
// (forgetOrigin() in src/popup/views/settings.js) filters an origin out of
// state[key][addr] in place, deleting the address key entirely once its list
// is empty — the exact membership-vs-whole-field pattern that made the
// whole-field `wallets` diff unsafe, on a security-relevant field: a stale
// whole-field save here can resurrect a revoked permission or wipe a freshly
// granted one.
const ADDR1 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ADDR2 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
function approveSite(pageState, address, hostname) {
function approveSite(pageState, address, origin) {
if (!pageState.allowedSites[address]) {
pageState.allowedSites[address] = [];
}
if (!pageState.allowedSites[address].includes(hostname)) {
pageState.allowedSites[address].push(hostname);
if (!pageState.allowedSites[address].includes(origin)) {
pageState.allowedSites[address].push(origin);
}
}
function revokeSite(pageState, hostname) {
function revokeSite(pageState, origin) {
for (const addr of Object.keys(pageState.allowedSites)) {
pageState.allowedSites[addr] = pageState.allowedSites[addr].filter(
(h) => h !== hostname,
(o) => o !== origin,
);
if (pageState.allowedSites[addr].length === 0) {
delete pageState.allowedSites[addr];
@@ -308,7 +309,7 @@ describe("a dApp approval racing a stale Settings page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR2]: ["other.example"] },
allowedSites: { [ADDR2]: ["https://other.example"] },
},
});
@@ -318,24 +319,24 @@ describe("a dApp approval racing a stale Settings page's later save", () => {
await settings.state.loadState();
// A dApp approval window, opened later, approves a new site for a
// different address and saves — the real sequence at
// src/background/index.js:592-599.
// different address and saves — the real sequence in
// rememberSiteChoice(), src/background/index.js.
const approval = loadPage(storage);
await approval.state.loadState();
approveSite(approval.state.state, ADDR1, "dapp.example");
approveSite(approval.state.state, ADDR1, "https://dapp.example");
await approval.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toEqual(["dapp.example"]);
).toEqual(["https://dapp.example"]);
// Settings revokes its own, unrelated site — the real sequence at
// src/popup/views/settings.js:55-68 — and saves from state loaded
// before the dApp approval ever happened.
revokeSite(settings.state.state, "other.example");
// Settings revokes its own, unrelated site — the real sequence in
// forgetOrigin(), src/popup/views/settings.js — and saves from state
// loaded before the dApp approval ever happened.
revokeSite(settings.state.state, "https://other.example");
await settings.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR1]).toEqual(["dapp.example"]);
expect(persisted.allowedSites[ADDR1]).toEqual(["https://dapp.example"]);
expect(persisted.allowedSites[ADDR2]).toBeUndefined();
});
});
@@ -346,7 +347,7 @@ describe("a revoked site permission against a stale page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR1]: ["evil.example"] },
allowedSites: { [ADDR1]: ["https://evil.example"] },
},
});
@@ -354,23 +355,24 @@ describe("a revoked site permission against a stale page's later save", () => {
const stale = loadPage(storage);
await stale.state.loadState();
// Settings revokes it — src/popup/views/settings.js:55-68 — from a
// second page.
// Settings revokes it — forgetOrigin(), src/popup/views/settings.js —
// from a second page.
const settings = loadPage(storage);
await settings.state.loadState();
revokeSite(settings.state.state, "evil.example");
revokeSite(settings.state.state, "https://evil.example");
await settings.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toBeUndefined();
// The stale page, unaware of the revoke, approves an unrelated site
// for a different address and saves — src/background/index.js:592-599.
approveSite(stale.state.state, ADDR2, "good.example");
// for a different address and saves — rememberSiteChoice(),
// src/background/index.js.
approveSite(stale.state.state, ADDR2, "https://good.example");
await stale.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR2]).toEqual(["good.example"]);
expect(persisted.allowedSites[ADDR2]).toEqual(["https://good.example"]);
expect(persisted.allowedSites[ADDR1]).toBeUndefined();
});
});
+3 -1
View File
@@ -167,7 +167,9 @@ describe("an unversioned profile that is perfectly valid", () => {
expect(stored.wallets[0].encryptedSecret).toBe("encrypted-secret-1");
expect(stored.wallets[0].addresses[0].address).toBe(ADDRESS);
expect(stored.activeAddress).toBe(ADDRESS);
expect(stored.allowedSites).toEqual({ [ADDRESS]: ["dapp.example"] });
expect(stored.allowedSites).toEqual({
[ADDRESS]: ["https://dapp.example"],
});
});
});
+1 -1
View File
@@ -71,7 +71,7 @@ function unversionedValidProfile(extra) {
networkId: "mainnet",
rpcUrl: "https://ethereum-rpc.publicnode.com",
blockscoutUrl: "https://eth.blockscout.com/api/v2",
allowedSites: { [ADDRESS]: ["dapp.example"] },
allowedSites: { [ADDRESS]: ["https://dapp.example"] },
deniedSites: {},
trackedTokens: [],
theme: "system",
+1 -1
View File
@@ -471,7 +471,7 @@ async function openSignScreen(data) {
if (msg.type !== "AUTISTMASK_GET_APPROVAL") return reply(null);
reply({
type: "sign",
hostname: "dapp.example",
origin: "https://dapp.example",
isPhishingDomain: false,
approvedFrom: OWNER,
signParams: request(data),
+20 -11
View File
@@ -28,11 +28,14 @@ function makeState(overrides = {}) {
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["a.example"],
[A1]: ["b.example"],
[B0]: ["c.example"],
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
[B0]: ["https://c.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[C0]: ["https://e.example"],
},
deniedSites: { [A1]: ["d.example"], [C0]: ["e.example"] },
...overrides,
};
}
@@ -41,7 +44,7 @@ describe("removeWalletFromState", () => {
test("deleting the last wallet clears hasWallet", () => {
const state = makeState({
wallets: [wallet("A", [A0])],
allowedSites: { [A0]: ["a.example"] },
allowedSites: { [A0]: ["https://a.example"] },
deniedSites: {},
});
@@ -109,8 +112,8 @@ describe("removeWalletFromState", () => {
removeWalletFromState(state, 0);
expect(state.allowedSites).toEqual({ [B0]: ["c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["e.example"] });
expect(state.allowedSites).toEqual({ [B0]: ["https://c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["https://e.example"] });
});
});
@@ -126,8 +129,14 @@ function makeAddressState(overrides = {}) {
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: { [A0]: ["a.example"], [A1]: ["b.example"] },
deniedSites: { [A1]: ["d.example"], [B0]: ["e.example"] },
allowedSites: {
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[B0]: ["https://e.example"],
},
...overrides,
};
}
@@ -273,8 +282,8 @@ describe("removeAddressFromState", () => {
removeAddressFromState(state, 0, 1);
expect(state.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["e.example"] });
expect(state.allowedSites).toEqual({ [A0]: ["https://a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["https://e.example"] });
});
// The derivation counter is a high-water mark, never rewound: "+" derives