fix: floor the persisted fields a restore dereferences, and make each field's floor an executable claim (closes #362)
A persisted container was checked while its ENTRIES were dereferenced
unchecked. A stored `{"0x…": "notalist"}` in allowedSites passes the state
gate, renders a working popup, and then throws inside saveState()'s per-
hostname merge, so every save from that moment on fails while the UI looks
entirely healthy. deniedSites has the identical shape; fraudContracts is the
same class with a milder consequence.
The sweep for that class found four more:
- selectedToken, dereferenced as text behind a truthiness-only restore gate.
- rpcUrl, handed whole to `new JsonRpcProvider()` by getProvider(), which
throws SYNCHRONOUSLY for a non-string — from txStatus.js and addWallet.js,
neither inside a try, and the first reachable from a stored
`currentView: "wait-tx"` through the unguarded restoreView().
- The ENTRIES of viewData. Four restore branches gate on one truthy field and
hand the rest to a renderer that calls address.toLowerCase(): a stored
`{"currentView":"success-tx","viewData":{"hash":"0x1"}}` throws out of
restoreView(), skipping the rest of popup init.
- selectedWallet / selectedAddress. `wallets` is a real Array, so a stored
"map", "length", "constructor" or "__proto__" is TRUTHY: hasValidAddress()'s
`&&` does not short-circuit and `.addresses[…]` throws. A stale INTEGER index
is the safe case.
Floors, in src/shared/persistedState.js: allowedSites/deniedSites through
siteMap(), fraudContracts and each hostname list through textList(),
selectedToken and activeAddress as text-or-null, rpcUrl and blockscoutUrl as
non-empty text, selectedWallet and selectedAddress as a non-negative integer
or null, and each networkEndpoints pair's two URL fields — which
applyChainSwitchFields() assigns straight onto s.rpcUrl on the next switch.
Guards, in src/popup/viewRouter.js: the four restore branches that gate on one
truthy field now check the entries their renderer dereferences, as
txStatus.restoreWait() has always done for wait-tx. "confirm-tx" joins
ADDRESS_VIEWS, because its Sign button dereferences
state.wallets[state.selectedWallet] behind no guard of its own.
A stored own "__proto__" key is dropped by siteMap(): it can never be a wallet
address, so it grants nothing, and keeping it only keeps a value the next save
would hand to the prototype setter. networkEndpoints keeps unknown keys by
design, so mergeMapByKey() in src/shared/state.js now writes with
defineProperty as well — the guard in the floor was being undone one layer
downstream.
A save that fails is also told, not merely repaired: onSaveFailure() reports
every failed save, awaited or not (the save queue's own rejection handler is
what made a failure vanish), and the popup raises a persistent "NOT SAVED"
banner naming the reason. doRefreshAndRender() no longer rejects, since every
one of its call sites fires it and walks away.
The per-field justification in the header of src/shared/stateSchema.js is
replaced by tests/persistedFieldContract.test.js. That comment shipped a false
claim in three consecutive changes; the artifact was the problem. The test is
one row per persisted field, declaring the property that field's floor is
claimed to have and PROVING it by driving the real code with hostile values —
the gate for a field the gate refuses, normalizePersisted() for a field it
floors, the real JsonRpcProvider constructor for rpcUrl, and a boot of the real
popup entry point for every field whose only defence is that nothing
dereferences it structurally. A field added to PERSISTED_FIELDS with no row
fails the suite; so does a row whose claim is false. The header and the README
mirror now point at it instead of restating it.
This commit is contained in:
404
tests/persistedEntryFloors.test.js
Normal file
404
tests/persistedEntryFloors.test.js
Normal file
@@ -0,0 +1,404 @@
|
||||
// A persisted container that is checked while its ENTRIES are dereferenced
|
||||
// unchecked (https://git.eeqj.de/sneak/AutistMask/issues/362).
|
||||
//
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/311 settled the idiom — floor the
|
||||
// container AND its entries, dropping anything that cannot be safely
|
||||
// dereferenced — and applied it to trackedTokens and tokenBalances. These are
|
||||
// the fields it did not reach.
|
||||
//
|
||||
// allowedSites is the worst shape in the codebase, and it is what the boot
|
||||
// tests below measure: a stored `{"0x…": "notalist"}` passes the gate, renders
|
||||
// a WORKING popup, and then throws `base.map is not a function` inside
|
||||
// saveState()'s merge — so every save from then on fails while the UI looks
|
||||
// entirely healthy and the user goes on operating a wallet that is persisting
|
||||
// nothing. A blank popup is at least visibly broken; this is not. So the
|
||||
// assertion here is never merely "the popup rendered": it is "the popup
|
||||
// rendered AND the write actually landed in storage".
|
||||
//
|
||||
// Observed at ad6aa7b, with the floors below removed:
|
||||
// allowedSites: {"0x…": "notalist"} -> views=["main"], errors=[], and
|
||||
// storage.set NEVER called: the stored record kept no schemaVersion, so
|
||||
// nothing the user did was persisted.
|
||||
// fraudContracts: "0x…" -> renderSendTokenSelect() threw
|
||||
// "(state.fraudContracts || []).map is not a function"
|
||||
// fraudContracts: [42] -> threw "a.toLowerCase is not a
|
||||
// function"
|
||||
// selectedToken: 42 (restoring onto address-token) -> views=[], errors=
|
||||
// ["tokenId.toLowerCase is not a function"] — a blank popup.
|
||||
|
||||
const { normalizePersisted } = require("../src/shared/persistedState");
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
const {
|
||||
bootPopup,
|
||||
cleanupPopup,
|
||||
unversionedValidProfile,
|
||||
ADDRESS,
|
||||
TOKEN_ADDRESS,
|
||||
} = require("./support/popupBoot");
|
||||
|
||||
// One extension page: a fresh module registry over the given storage. state.js
|
||||
// resolves the storage API at require time, so the stub has to be installed
|
||||
// before the module is loaded.
|
||||
function loadStateModule(storage) {
|
||||
jest.resetModules();
|
||||
globalThis.chrome = { storage: { local: storage.local } };
|
||||
return require("../src/shared/state");
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanupPopup();
|
||||
});
|
||||
|
||||
// ------------------------------------------------------- the floor itself
|
||||
|
||||
describe("the floor under allowedSites and deniedSites", () => {
|
||||
for (const field of ["allowedSites", "deniedSites"]) {
|
||||
test(`${field} that is not a record becomes an empty record`, () => {
|
||||
for (const bad of ["nope", 42, true, [ADDRESS], null]) {
|
||||
expect(normalizePersisted({ [field]: bad })[field]).toEqual({});
|
||||
}
|
||||
});
|
||||
|
||||
test(`an ${field} entry whose value is not a hostname list is dropped`, () => {
|
||||
for (const bad of ["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`, () => {
|
||||
expect(
|
||||
normalizePersisted({
|
||||
[field]: { [ADDRESS]: [42, null, "dapp.example", {}] },
|
||||
})[field],
|
||||
).toEqual({ [ADDRESS]: ["dapp.example"] });
|
||||
});
|
||||
|
||||
test(`a real ${field} map survives, copied not shared`, () => {
|
||||
const saved = { [field]: { [ADDRESS]: ["dapp.example"] } };
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(out[field]).toEqual(saved[field]);
|
||||
expect(out[field]).not.toBe(saved[field]);
|
||||
expect(out[field][ADDRESS]).not.toBe(saved[field][ADDRESS]);
|
||||
});
|
||||
|
||||
test(`a good ${field} entry beside a malformed one survives`, () => {
|
||||
const out = normalizePersisted({
|
||||
[field]: { [ADDRESS]: ["dapp.example"], [TOKEN_ADDRESS]: 42 },
|
||||
});
|
||||
|
||||
expect(out[field]).toEqual({ [ADDRESS]: ["dapp.example"] });
|
||||
});
|
||||
|
||||
test(`a stored own "__proto__" key in ${field} is dropped`, () => {
|
||||
// JSON can carry the key. It can never be a wallet address, so it
|
||||
// grants and denies nothing and goes the way of every other key
|
||||
// whose value is unusable; keeping it would only keep a value that
|
||||
// saveState()'s merge hands to the prototype setter on the next
|
||||
// write.
|
||||
const saved = JSON.parse(
|
||||
'{"' + field + '":{"__proto__":["evil.invalid"]}}',
|
||||
);
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(Object.getPrototypeOf(out[field])).toBe(Object.prototype);
|
||||
expect(Object.keys(out[field])).toEqual([]);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('a stored own "__proto__" key surviving a save', () => {
|
||||
// The floor writes map keys with defineProperty; saveState()'s merge is one
|
||||
// layer downstream of it and used to write them with plain assignment,
|
||||
// which hands "__proto__" to the prototype setter and records no entry.
|
||||
// networkEndpoints is where a key that is not a known id is deliberately
|
||||
// KEPT, so it is where that undoing shows.
|
||||
test("does not move a prototype or vanish from networkEndpoints", async () => {
|
||||
const profile = unversionedValidProfile();
|
||||
profile.networkEndpoints = JSON.parse(
|
||||
'{"__proto__":{"rpcUrl":"https://kept.invalid"}}',
|
||||
);
|
||||
const storage = makeStorageStub({ autistmask: profile });
|
||||
const { state, loadState, saveState } = loadStateModule(storage);
|
||||
|
||||
await loadState();
|
||||
state.theme = "dark";
|
||||
await saveState();
|
||||
|
||||
// Not compared against Object.prototype by identity: the storage stub
|
||||
// clones through structuredClone, which builds the result in the host
|
||||
// realm, so the two Object.prototypes are different objects.
|
||||
const stored = storage.read("autistmask").networkEndpoints;
|
||||
expect(Object.getPrototypeOf(stored).rpcUrl).toBeUndefined();
|
||||
expect(Object.keys(stored)).toContain("__proto__");
|
||||
});
|
||||
});
|
||||
|
||||
describe("the floor under fraudContracts", () => {
|
||||
test("fraudContracts that is not a list becomes an empty list", () => {
|
||||
for (const bad of ["nope", 42, true, { a: 1 }]) {
|
||||
expect(
|
||||
normalizePersisted({ fraudContracts: bad }).fraudContracts,
|
||||
).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a fraudContracts entry that is not text is dropped", () => {
|
||||
expect(
|
||||
normalizePersisted({
|
||||
fraudContracts: [42, null, TOKEN_ADDRESS, {}, []],
|
||||
}).fraudContracts,
|
||||
).toEqual([TOKEN_ADDRESS]);
|
||||
});
|
||||
|
||||
test("a real fraudContracts list survives, copied not shared", () => {
|
||||
const saved = { fraudContracts: [TOKEN_ADDRESS] };
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(out.fraudContracts).toEqual(saved.fraudContracts);
|
||||
expect(out.fraudContracts).not.toBe(saved.fraudContracts);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the floor under selectedToken", () => {
|
||||
// Found by the sweep for this defect class, not named in the issue: the
|
||||
// restore gate in src/popup/viewRouter.js checks truthiness only, and both
|
||||
// src/popup/views/addressToken.js and src/popup/views/receive.js then
|
||||
// dereference it as text.
|
||||
test("a selectedToken that is not text becomes null", () => {
|
||||
for (const bad of [42, true, { a: 1 }, [TOKEN_ADDRESS]]) {
|
||||
expect(
|
||||
normalizePersisted({ selectedToken: bad }).selectedToken,
|
||||
).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test("a real selectedToken survives; the empty string becomes null", () => {
|
||||
expect(
|
||||
normalizePersisted({ selectedToken: TOKEN_ADDRESS }).selectedToken,
|
||||
).toBe(TOKEN_ADDRESS);
|
||||
expect(normalizePersisted({ selectedToken: "ETH" }).selectedToken).toBe(
|
||||
"ETH",
|
||||
);
|
||||
expect(
|
||||
normalizePersisted({ selectedToken: "" }).selectedToken,
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------- what the user actually gets
|
||||
|
||||
describe("a malformed allowedSites entry", () => {
|
||||
const MALFORMED = [
|
||||
{ name: "a string", value: "notalist" },
|
||||
{ name: "a number", value: 42 },
|
||||
{ name: "a record", value: { hostnames: ["dapp.example"] } },
|
||||
];
|
||||
|
||||
for (const { name, value } of MALFORMED) {
|
||||
test(`whose value is ${name}: a working popup whose writes persist`, async () => {
|
||||
const env = await bootPopup(
|
||||
unversionedValidProfile({
|
||||
allowedSites: { [ADDRESS]: value },
|
||||
}),
|
||||
);
|
||||
|
||||
expect({
|
||||
visibleViews: env.visibleViews(),
|
||||
errors: env.pageErrors,
|
||||
}).toEqual({ visibleViews: ["main"], errors: [] });
|
||||
|
||||
// The half that matters. A popup that renders and never persists
|
||||
// again is worse than one that renders nothing, because nothing
|
||||
// tells the user. The version stamp is proof a write landed: it
|
||||
// is absent from the stored record until saveState() writes one.
|
||||
expect(env.storage.set).toHaveBeenCalled();
|
||||
const stored = env.storage.read("autistmask");
|
||||
expect(stored.schemaVersion).toBe(1);
|
||||
expect(stored.wallets[0].encryptedSecret).toBe(
|
||||
"encrypted-secret-1",
|
||||
);
|
||||
expect(stored.allowedSites).toEqual({});
|
||||
});
|
||||
}
|
||||
|
||||
test("the well-formed entries beside it keep working", async () => {
|
||||
const env = await bootPopup(
|
||||
unversionedValidProfile({
|
||||
allowedSites: {
|
||||
[ADDRESS]: ["dapp.example"],
|
||||
[TOKEN_ADDRESS]: "notalist",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(env.pageErrors).toEqual([]);
|
||||
expect(env.storage.read("autistmask").allowedSites).toEqual({
|
||||
[ADDRESS]: ["dapp.example"],
|
||||
});
|
||||
});
|
||||
|
||||
test("a later save still lands, not just the first", async () => {
|
||||
// The failure this closes was in the MERGE, which runs on every save
|
||||
// against whatever is in storage at the time. One write landing is not
|
||||
// enough: the field has to stay mergeable.
|
||||
const storage = makeStorageStub({
|
||||
autistmask: unversionedValidProfile({
|
||||
allowedSites: { [ADDRESS]: "notalist" },
|
||||
}),
|
||||
});
|
||||
const { state, loadState, saveState } = loadStateModule(storage);
|
||||
|
||||
await loadState();
|
||||
state.theme = "dark";
|
||||
await saveState();
|
||||
state.utcTimestamps = true;
|
||||
await saveState();
|
||||
|
||||
const stored = storage.read("autistmask");
|
||||
expect(stored.theme).toBe("dark");
|
||||
expect(stored.utcTimestamps).toBe(true);
|
||||
expect(stored.allowedSites).toEqual({});
|
||||
expect(stored.wallets[0].encryptedSecret).toBe("encrypted-secret-1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("a malformed fraudContracts", () => {
|
||||
// The send screen, which is where this one lands: the boot path only
|
||||
// reaches fraudContracts through loadHomeTxs(), which catches, so the
|
||||
// consequence is an unusable send screen rather than silent data loss.
|
||||
function stubSendDocument() {
|
||||
const select = { innerHTML: "", children: [] };
|
||||
select.appendChild = (child) => select.children.push(child);
|
||||
globalThis.document = {
|
||||
getElementById: (id) => (id === "send-token" ? select : null),
|
||||
createElement: () => ({ value: "", textContent: "" }),
|
||||
};
|
||||
return select;
|
||||
}
|
||||
|
||||
const HELD = {
|
||||
address: TOKEN_ADDRESS,
|
||||
symbol: "AAA",
|
||||
decimals: 18,
|
||||
balance: "12.5",
|
||||
holders: 50000,
|
||||
};
|
||||
|
||||
async function sendScreenTokens(fraudContracts) {
|
||||
const storage = makeStorageStub({
|
||||
autistmask: unversionedValidProfile({ fraudContracts }),
|
||||
});
|
||||
const { loadState } = loadStateModule(storage);
|
||||
await loadState();
|
||||
const select = stubSendDocument();
|
||||
const { renderSendTokenSelect } = require("../src/popup/views/send");
|
||||
|
||||
renderSendTokenSelect({ address: ADDRESS, tokenBalances: [HELD] });
|
||||
|
||||
return select.children.map((opt) => opt.value);
|
||||
}
|
||||
|
||||
for (const bad of ["notalist", 42, { a: 1 }, [42], [null], [{}]]) {
|
||||
test(`${JSON.stringify(bad)}: a usable send screen`, async () => {
|
||||
await expect(sendScreenTokens(bad)).resolves.toEqual([
|
||||
TOKEN_ADDRESS,
|
||||
]);
|
||||
});
|
||||
}
|
||||
|
||||
test("a real fraud entry beside a malformed one still hides its token", async () => {
|
||||
await expect(
|
||||
sendScreenTokens([42, TOKEN_ADDRESS.toLowerCase()]),
|
||||
).resolves.toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("a malformed selectedToken", () => {
|
||||
test("does not blank the popup on restore", async () => {
|
||||
const env = await bootPopup(
|
||||
unversionedValidProfile({
|
||||
currentView: "address-token",
|
||||
selectedWallet: 0,
|
||||
selectedAddress: 0,
|
||||
selectedToken: 42,
|
||||
viewStack: ["main", "address"],
|
||||
}),
|
||||
);
|
||||
|
||||
expect({
|
||||
visibleViews: env.visibleViews(),
|
||||
errors: env.pageErrors,
|
||||
}).toEqual({ visibleViews: ["main"], errors: [] });
|
||||
});
|
||||
});
|
||||
|
||||
// --------------------------------------------- a save that fails is told
|
||||
|
||||
describe("a save that fails", () => {
|
||||
function failingStorage(profile) {
|
||||
const storage = makeStorageStub({ autistmask: profile });
|
||||
const realSet = storage.local.set;
|
||||
storage.local.set = jest.fn(async () => {
|
||||
throw new Error("QUOTA_BYTES quota exceeded");
|
||||
});
|
||||
storage.restoreWrites = () => {
|
||||
storage.local.set = realSet;
|
||||
};
|
||||
return storage;
|
||||
}
|
||||
|
||||
test("is reported, not swallowed by the save queue", async () => {
|
||||
const storage = failingStorage(unversionedValidProfile());
|
||||
const { state, loadState, saveState, onSaveFailure } =
|
||||
loadStateModule(storage);
|
||||
const failures = [];
|
||||
onSaveFailure((e) => failures.push(String(e && e.message)));
|
||||
|
||||
await loadState();
|
||||
state.theme = "dark";
|
||||
// Not awaited, which is how showView() saves on every navigation and
|
||||
// how the failure used to disappear entirely.
|
||||
saveState();
|
||||
for (let i = 0; i < 50; i++) await Promise.resolve();
|
||||
|
||||
expect(failures).toEqual(["QUOTA_BYTES quota exceeded"]);
|
||||
});
|
||||
|
||||
test("still rejects for a caller that awaits it", async () => {
|
||||
const storage = failingStorage(unversionedValidProfile());
|
||||
const { state, loadState, saveState, onSaveFailure } =
|
||||
loadStateModule(storage);
|
||||
onSaveFailure(() => {});
|
||||
|
||||
await loadState();
|
||||
state.theme = "dark";
|
||||
|
||||
await expect(saveState()).rejects.toThrow("QUOTA_BYTES");
|
||||
});
|
||||
|
||||
test("puts a banner on the popup saying nothing is being saved", async () => {
|
||||
const env = await bootPopup(undefined, {
|
||||
storage: failingStorage(unversionedValidProfile()),
|
||||
});
|
||||
|
||||
// The popup is still usable — the point is that it no longer looks
|
||||
// healthy while silently persisting nothing.
|
||||
expect(env.visibleViews()).toEqual(["main"]);
|
||||
const banner = env.node("save-failure-banner");
|
||||
expect(banner).not.toBeNull();
|
||||
expect(banner.textContent).toContain("NOT SAVED");
|
||||
expect(banner.textContent).toContain("QUOTA_BYTES quota exceeded");
|
||||
});
|
||||
|
||||
test("no banner appears on a popup whose saves work", async () => {
|
||||
const env = await bootPopup(unversionedValidProfile());
|
||||
|
||||
expect(env.node("save-failure-banner")).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user