fix: version stored state, validate its shape, and give a corrupt blob a way out (closes #311)
All checks were successful
check / check (push) Successful in 33s
e2e / e2e-chrome (push) Successful in 1m46s
e2e / e2e-firefox (push) Successful in 34s

Stored state had no version and no structural validation, so a corrupt blob produced a completely blank popup with no message and no recovery control, and made every dApp RPC call from every page answer a generic -32603. There was no reset or wipe control anywhere in the UI.

saveState() now stamps a schema version and loadState() validates the shape. A version it does not understand, or a wallets array it cannot parse, lands on a recovery screen that names the problem, offers the stored record verbatim for export, and offers a destructive reset behind a typed confirmation. Unversioned but valid state -- which every existing install has -- migrates in place and keeps working; it is never shown a wipe prompt. A dApp call against unusable state answers -32007, which EIP-1474 leaves unassigned, rather than -32603. networkById() refuses an unknown id loudly instead of returning mainnet, and networkId is validated so a corrupt value cannot be used as an object key.

Fields the gate does not refuse are floored by type, container and entries both: a malformed trackedTokens or tokenBalances entry is dropped rather than dereferenced. Verified by an independent sweep of 1152 corrupt blobs producing no blank popup, with the same harness showing 9 blanks against the previous revision.
This commit was merged in pull request #360.
This commit is contained in:
2026-08-23 20:04:00 +02:00
parent 28a527295a
commit ad6aa7b20d
25 changed files with 2270 additions and 50 deletions

View File

@@ -194,6 +194,23 @@ function storageSet(items) {
return Promise.resolve(storage.set(items));
}
/**
* Erase stored keys. The one caller is the destructive reset on the recovery
* screen (src/popup/views/stateRecovery.js), which is the only way out of a
* profile no build can read; it rejects rather than defaulting for the same
* reason the two above do — a reset that silently did nothing would leave the
* user in the dead end they were promised an exit from.
*
* @param {string|string[]} keys
* @returns {Promise<void>}
* @throws rejects where `storage.local` is absent.
*/
function storageRemove(keys) {
const storage = storageLocal();
if (!storage) return storageUnavailable("remove");
return Promise.resolve(storage.remove(keys));
}
/**
* @param {Object} queryInfo
* @returns {Promise<Array>} the matching tabs.
@@ -251,6 +268,7 @@ module.exports = {
sendMessage,
storageGet,
storageLocal,
storageRemove,
storageSet,
tabsApi,
tabsQuery,

View File

@@ -31,8 +31,42 @@ const SUPPORTED_CHAIN_IDS = new Set(
Object.values(NETWORKS).map((n) => n.chainId),
);
// Thrown rather than defaulted. An id this build does not know used to answer
// with MAINNET, so a stored `{networkId:"base"}` rendered the selector as
// Ethereum Mainnet with no banner and answered eth_chainId 0x1, while rpcUrl
// still pointed at Base — the wallet telling the user and the page one chain
// while transacting on another. Nothing in this codebase has an unknown id to
// offer: stored state is validated against this table before it is loaded
// (src/shared/stateSchema.js), and every other caller passes an id it took
// from here. So an unknown id is a defect, and it says so, the same way
// getProvider() (src/shared/balances.js) already refuses one.
class UnknownNetworkError extends Error {
constructor(id) {
super(
"AutistMask does not know the network " +
JSON.stringify(id) +
"; it supports " +
Object.keys(NETWORKS).join(", "),
);
this.name = "UnknownNetworkError";
this.networkId = id;
}
}
// Own properties only: NETWORKS inherits from Object.prototype, so
// NETWORKS["constructor"] and NETWORKS["__proto__"] both answer with something
// truthy that is not a network. A stored id is untrusted input, and this is
// the test the validator uses to decide whether it may be adopted at all.
function isKnownNetworkId(id) {
return (
typeof id === "string" &&
Object.prototype.hasOwnProperty.call(NETWORKS, id)
);
}
function networkById(id) {
return NETWORKS[id] || NETWORKS.mainnet;
if (!isKnownNetworkId(id)) throw new UnknownNetworkError(id);
return NETWORKS[id];
}
function networkByChainId(chainId) {
@@ -51,6 +85,8 @@ function explorerLink(network, type, value) {
module.exports = {
NETWORKS,
SUPPORTED_CHAIN_IDS,
UnknownNetworkError,
isKnownNetworkId,
networkById,
networkByChainId,
explorerLink,

View File

@@ -10,8 +10,14 @@
// reaching it anyway and being served DEFAULT_STATE.
const { DEFAULT_RPC_URL, DEFAULT_BLOCKSCOUT_URL } = require("./constants");
// Dependency-free constant module; safe to pull into a background bundle.
const { RESTORABLE_VIEWS } = require("../popup/restorableViews");
const { isKnownNetworkId } = require("./networks");
const { STATE_SCHEMA_VERSION } = require("./stateSchema");
// Dependency-free constant module. It lives under src/shared/ rather than
// src/popup/ precisely because this module is in the background bundle: a
// popup-path module reached from the worker is the shape the prohibition in
// script/lib/forbiddenBundleInputs.js exists to keep out, even when the
// particular module is harmless.
const { RESTORABLE_VIEWS } = require("./restorableViews");
const DEFAULT_STATE = {
hasWallet: false,
@@ -46,7 +52,10 @@ const DEFAULT_STATE = {
// Every field written to and read from the single "autistmask" storage key.
// hasWallet is deliberately excluded from the diffing/merge logic in
// state.js — like loadState() does, it is always derived from `wallets`,
// never carried as an independent value.
// never carried as an independent value. schemaVersion is excluded for the
// same reason and is absent from DEFAULT_STATE for it: it describes the
// record rather than being part of it, and every write stamps the current
// value rather than diffing whatever was read.
const PERSISTED_FIELDS = Object.keys(DEFAULT_STATE)
.filter((key) => key !== "hasWallet")
.concat([
@@ -58,6 +67,31 @@ const PERSISTED_FIELDS = Object.keys(DEFAULT_STATE)
"viewStack",
]);
function isRecord(value) {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
// A list of token references, as everything downstream dereferences them:
// `t.address.toLowerCase()`, with no guard of its own (src/shared/balances.js,
// src/popup/views/helpers.js, and every view that shows a balance line).
//
// Both the container AND the entries, because they are separate defects. A
// container check alone leaves a well-formed list of malformed entries walking
// through to a dereference one level below the check, which is the same blank
// popup: `[1, 2]` and `[{}]` are lists.
//
// A malformed entry is DROPPED rather than repaired: a token reference with no
// address identifies nothing, so there is no value to repair it to, and the
// alternative — refusing the whole record — sends a user whose wallets are
// perfectly readable to an export-or-erase screen over a token list. An entry
// that is a record with a text address is kept verbatim, extra fields and all.
function tokenRefs(value) {
if (!Array.isArray(value)) return [];
return value.filter(
(entry) => isRecord(entry) && typeof entry.address === "string",
);
}
// Keep only the leading run of stored views the popup is willing to render.
//
// restoreView() refuses to reopen ONTO a non-restorable view, but the stack
@@ -107,11 +141,52 @@ function restorableStack(stored, currentView) {
function normalizePersisted(saved) {
saved = saved || {};
const out = {};
// Every write goes out at the current version. That IS the migration for
// the unversioned records every install in the field holds: version 1 is
// the shape that shipped unversioned, so a record that validated is
// carried forward simply by being stamped. A record this build does NOT
// understand never reaches here — assertStateUsable() refuses it on the
// read path first (src/shared/stateSchema.js).
out.schemaVersion = STATE_SCHEMA_VERSION;
out.wallets = structuredClone(saved.wallets || []);
// Derived, never trusted verbatim off storage — see loadState().
out.hasWallet = out.wallets.length > 0;
out.trackedTokens = structuredClone(saved.trackedTokens || []);
out.networkId = saved.networkId || DEFAULT_STATE.networkId;
// Each address's token holdings, floored to a list of token records on the
// detached copy above. Every reader iterates it behind a `|| []` that only
// covers an ABSENT value, and then dereferences `t.address.toLowerCase()`
// and `t.balance` — so a stored string iterates as characters, a number
// throws on the iterator, and a null entry throws on the field.
//
// This field specifically, because refreshBalances() writes it WHOLESALE
// rather than merging into it: a write that only partly lands is the live
// cause https://git.eeqj.de/sneak/AutistMask/issues/311 names, and this is
// where it lands. The wallet list itself is the gate's (stateSchema.js);
// what is below an address record is not, and gets floored here.
if (Array.isArray(out.wallets)) {
for (const wallet of out.wallets) {
if (!isRecord(wallet) || !Array.isArray(wallet.addresses)) continue;
for (const addr of wallet.addresses) {
if (!isRecord(addr)) continue;
addr.tokenBalances = tokenRefs(addr.tokenBalances);
}
}
}
// An actual list of token records is required, not merely a truthy value
// and not merely a list: everything downstream iterates this and
// dereferences `token.address`, so a stored string or object walks through
// a `|| []`, and a list of numbers walks through an Array.isArray(), and
// both throw on the first read — the blank popup from the issue, for a
// profile whose wallets are perfectly fine. An empty list is a legitimate
// value and survives.
out.trackedTokens = structuredClone(tokenRefs(saved.trackedTokens));
// The loud refusal for an unknown id is assertStateUsable(); this is the
// floor under it. networkId is an object KEY into networkEndpoints below,
// so a value that is not a network in networks.js must never get that far
// — "__proto__" would set the map's prototype instead of an own key, and
// the user's endpoint would silently not be recorded.
out.networkId = isKnownNetworkId(saved.networkId)
? saved.networkId
: DEFAULT_STATE.networkId;
out.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl;
out.blockscoutUrl = saved.blockscoutUrl || DEFAULT_STATE.blockscoutUrl;
// An actual object is required, not merely a truthy non-array: the code
@@ -127,7 +202,19 @@ function normalizePersisted(saved) {
: {};
out.networkEndpoints = {};
for (const netId of Object.keys(rawEndpoints)) {
out.networkEndpoints[netId] = { ...rawEndpoints[netId] };
// defineProperty, not assignment: a stored map with an own
// "__proto__" key — which JSON can carry and assignment treats as the
// prototype setter — would otherwise replace this object's prototype
// and record no entry at all. Keys other than the known network ids
// are kept rather than dropped, so a profile that has been on a build
// with more networks does not lose their endpoints by passing through
// this one.
Object.defineProperty(out.networkEndpoints, netId, {
value: { ...rawEndpoints[netId] },
writable: true,
enumerable: true,
configurable: true,
});
}
// A profile written before this map existed carries exactly one pair of
// endpoints, belonging to whatever network it was last on. Adopt it as
@@ -140,7 +227,18 @@ function normalizePersisted(saved) {
};
}
out.lastBalanceRefresh = saved.lastBalanceRefresh || 0;
out.activeAddress = saved.activeAddress || null;
// A non-empty address, or null, never anything else: this is passed to
// address.slice() and compared against stored addresses, so a stored
// number or object walks through a `|| null` and throws on the first
// render. The empty string is text but it is not an address, and it must
// become null rather than survive: init() auto-selects the first address
// only on a STRICT null, so a stored "" would leave the popup with no
// address ever selected. Nothing in src/ writes one, and this keeps the
// behaviour the `|| null` this check replaced already had.
out.activeAddress =
typeof saved.activeAddress === "string" && saved.activeAddress !== ""
? saved.activeAddress
: null;
out.allowedSites =
saved.allowedSites && !Array.isArray(saved.allowedSites)
? structuredClone(saved.allowedSites)

View File

@@ -0,0 +1,41 @@
// Views the popup may reopen onto.
//
// The popup persists the current view so that reopening the toolbar popup
// lands the user back where they were. Only views that can be fully
// re-rendered from persisted state belong here; every other view falls back
// to the nearest restorable parent (src/popup/index.js restoreView()).
//
// A view that displays a secret must NEVER be listed. Restoring onto one
// would put a private key or a recovery phrase on screen with no password
// prompt in front of it, on a popup the user may have reopened by accident.
// That is why "export-privkey" and "show-phrase" are absent.
//
// Nor may a view whose button destroys a wallet be listed, for the mirror
// reason: a popup reopened by accident must not land on the screen that
// erases key material. That is why "delete-wallet-confirm" and
// "delete-wallet-lost-password" are absent.
//
// Kept in its own module, with no dependencies, so tests can assert the
// exclusion directly rather than trusting a reading of the popup entry
// point.
//
// It sits under src/shared/ rather than src/popup/ because
// src/shared/persistedState.js needs it and that module is in the BACKGROUND
// bundle: a popup-path module reached from the worker is the shape
// script/lib/forbiddenBundleInputs.js exists to keep out, whether or not the
// particular module is harmless.
const RESTORABLE_VIEWS = new Set([
"main",
"address",
"address-token",
"receive",
"settings",
"settings-addtoken",
"confirm-tx",
"transaction",
"wait-tx",
"success-tx",
"error-tx",
]);
module.exports = { RESTORABLE_VIEWS };

View File

@@ -29,6 +29,12 @@ const {
normalizePersisted,
} = require("./persistedState");
const {
STATE_SCHEMA_VERSION,
assertStateUsable,
migrationNeeded,
} = require("./stateSchema");
const { storageGet, storageSet } = require("./browserApi");
const { log } = require("./log");
@@ -446,6 +452,14 @@ function mergeNetworkEndpoints(base, ours, theirs) {
async function saveStateOnce() {
const current = snapshotPersisted();
const result = await storageGet("autistmask");
// The record in storage right now is about to be merged into and written
// back, so it is validated exactly like a load validates it. Without this,
// a page whose own load succeeded would normalize a record it does not
// understand — one a NEWER build wrote in the meantime, say — and write
// the result back over it, destroying the only copy of whatever that
// record held. Refusing is louder than that and loses nothing: the live
// state is untouched and the next save retries.
assertStateUsable(result.autistmask);
// Normalized, not raw: a field this page did not change still has to
// come from storage in its loaded (self-healed) shape. See
// normalizePersisted() in persistedState.js.
@@ -481,6 +495,10 @@ async function saveStateOnce() {
}
}
merged.hasWallet = Boolean(merged.wallets && merged.wallets.length > 0);
// Stamped on every write, never merged or diffed: the record that goes to
// storage is in THIS build's shape whatever shape it was read in, which is
// what migrates the unversioned records every install in the field holds.
merged.schemaVersion = STATE_SCHEMA_VERSION;
await storageSet({ autistmask: merged });
@@ -512,8 +530,24 @@ function saveState() {
return turn;
}
// Rejects with StateUnusableError for a stored record this build cannot make
// sense of. Nothing is assigned and `loaded` stays false in that case, so a
// caller that ignores the rejection gets StateNotLoadedError on the first
// read rather than a half-populated profile. The caller that does NOT ignore
// it is the popup entry point, which shows the recovery screen
// (src/popup/views/stateRecovery.js) instead of proceeding.
async function loadState() {
const result = await storageGet("autistmask");
// Before normalization, on the raw bytes: normalizing first would paper
// over the very shapes this refuses, which is how a corrupt record used to
// reach the popup and blank it (issue #311).
assertStateUsable(result.autistmask);
if (migrationNeeded(result.autistmask)) {
log.infof(
"state: migrating an unversioned profile to schema version",
STATE_SCHEMA_VERSION,
);
}
if (result.autistmask) {
Object.assign(rawState, normalizePersisted(result.autistmask));
}

265
src/shared/stateSchema.js Normal file
View File

@@ -0,0 +1,265 @@
// The version stamped on the stored profile, and the shape check every read
// of one goes through.
//
// Storage is the one input to this extension that nobody validated. A profile
// carried no version at all, so there was no way to tell a record this build
// understands from one a later build wrote, and loadState() coerced scalars
// while trusting the structure — so a `wallets` that was a string, or an array
// of nulls, or a later schema's wallet records, reached the popup and threw on
// the first dereference. The popup rendered NOTHING: no view, no message, no
// control, and no way out from inside the product
// (https://git.eeqj.de/sneak/AutistMask/issues/311).
//
// Two separate jobs, deliberately not merged:
//
// stateProblem() / assertStateUsable() refuse a record this build cannot
// safely reason about, loudly, naming the problem in a
// sentence that goes on screen. This is the gate.
// normalizePersisted() (persistedState.js) self-heal a record that IS
// usable: absent fields, legacy shapes, out-of-range flags.
//
// The gate runs FIRST, on the raw stored bytes, before normalization has a
// chance to paper over a record whose meaning nobody can vouch for. A blob
// that fails it is left in storage untouched — it is the user's only copy of
// whatever it holds, and the recovery screen exports it before offering to
// erase it.
//
// What is checked HERE is what nothing downstream can floor: the wallet list,
// the version, and the network id that keys an object. Every other field is
// normalizePersisted()'s to make safe, and what that function does today is
// NOT uniform. The four kinds of floor it applies, listed so a reader can tell
// which one a given field has without reading it off:
//
// Type-checked, container AND entries: trackedTokens, each address's
// tokenBalances, networkId, networkEndpoints, activeAddress, viewStack.
// These are the fields something dereferences structurally — iterated,
// indexed, assigned into, or .toLowerCase()'d — where a truthy value of
// the wrong type throws on the first read. The entries matter as much as
// the container: [1, 2] IS a list, and `t.address` is one level below the
// Array.isArray().
// Container shape only: allowedSites, deniedSites. A falsy value or a list
// becomes {}; anything else is taken as stored and the entries are not
// checked.
// `saved.x || default`, no type check: rpcUrl, blockscoutUrl,
// lastBalanceRefresh, fraudContracts, tokenHolderCache, theme,
// currentView, selectedToken, viewData.
// Present-or-default, value taken verbatim: every boolean flag,
// dustThresholdGwei, selectedWallet, selectedAddress.
//
// A field added to the record needs a check here or a floor there, chosen by
// what reads it: anything dereferenced structurally needs the type check, and
// neither of the last two kinds is one.
const { isKnownNetworkId } = require("./networks");
// Bump this when the MEANING of a stored field changes, and add the migration
// that carries the older version forward. Adding a field with a defaulted
// absent value is not a bump: normalizePersisted() already handles that, and
// bumping for it would send every older install to the recovery screen for no
// reason.
//
// Version 1 is the shape that shipped unversioned. An unversioned record is
// therefore version 1, not a defect — see migrationNeeded() below.
const STATE_SCHEMA_VERSION = 1;
// Thrown by every read path that finds a record it cannot use. `problem` is
// the sentence shown to the user; `message` carries the same text so a log
// line or a rethrow is not empty.
class StateUnusableError extends Error {
constructor(problem) {
super(problem);
this.name = "StateUnusableError";
this.problem = problem;
}
}
function isPlainObject(value) {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
// Own properties only, everywhere in this file. `saved` comes from storage as
// parsed JSON, so `saved.constructor` and `saved.__proto__` answer from the
// prototype chain for a record that carries neither — a check written as a
// plain truthiness test can be satisfied by Object.prototype rather than by
// anything the user's profile actually contains.
function has(obj, key) {
return Object.prototype.hasOwnProperty.call(obj, key);
}
function ordinal(index) {
return String(index + 1);
}
function describeType(value) {
if (value === null) return "null";
if (Array.isArray(value)) return "a list";
return "a " + typeof value;
}
// One address record, as every screen dereferences it.
function addressProblem(addr, walletIndex, addrIndex) {
const where =
"address " +
ordinal(addrIndex) +
" of wallet " +
ordinal(walletIndex) +
" in the saved data";
if (!isPlainObject(addr)) {
return "The " + where + " is " + describeType(addr) + ", not a record.";
}
if (typeof addr.address !== "string" || addr.address === "") {
return "The " + where + " has no address.";
}
return null;
}
function walletProblem(wallet, index) {
const where = "Wallet " + ordinal(index) + " in the saved data";
if (!isPlainObject(wallet)) {
return where + " is " + describeType(wallet) + ", not a wallet record.";
}
if (!Array.isArray(wallet.addresses)) {
return where + " has no list of addresses.";
}
if (has(wallet, "name") && typeof wallet.name !== "string") {
return where + " has a name that is not text.";
}
for (let i = 0; i < wallet.addresses.length; i++) {
const problem = addressProblem(wallet.addresses[i], index, i);
if (problem) return problem;
}
return null;
}
function versionProblem(saved) {
// No version field at all is the shape every install in the field has:
// no build ever wrote one. It is version 1, and it is migrated in place.
if (!has(saved, "schemaVersion")) return null;
const version = saved.schemaVersion;
if (
typeof version !== "number" ||
!Number.isInteger(version) ||
version < 1
) {
return (
"The saved data carries a schema version AutistMask does not" +
" recognize (" +
JSON.stringify(version) +
")."
);
}
if (version > STATE_SCHEMA_VERSION) {
return (
"The saved data was written by a newer version of AutistMask" +
" (schema version " +
version +
"; this build understands version " +
STATE_SCHEMA_VERSION +
")."
);
}
return null;
}
/**
* The reason this build cannot use `saved`, as a sentence for the user, or
* null when it can.
*
* @param {*} saved the raw record from storage, or undefined for a fresh
* install.
* @returns {string|null}
*/
function stateProblem(saved) {
// Nothing stored is a first run, not a defect.
if (saved === undefined || saved === null) return null;
if (!isPlainObject(saved)) {
return (
"The saved data is " +
describeType(saved) +
", not the record AutistMask stores."
);
}
const version = versionProblem(saved);
if (version) return version;
// Read once, from an OWN property or not at all, so that a polluted
// prototype cannot decide whether a profile is refused. Note that
// normalizePersisted() reads the same field plainly, and so WOULD consult
// the prototype chain: the two halves agree only because a record arriving
// from storage has been through structuredClone and always carries
// Object.prototype. Nothing reachable from storage can put them at odds,
// but a caller that hands either one a hand-built object with an unusual
// prototype is not covered by that.
const wallets =
has(saved, "wallets") && saved.wallets !== undefined
? saved.wallets
: [];
if (!Array.isArray(wallets)) {
return (
"The list of wallets in the saved data is " +
describeType(wallets) +
", not a list."
);
}
for (let i = 0; i < wallets.length; i++) {
const problem = walletProblem(wallets[i], i);
if (problem) return problem;
}
// networkId is not merely displayed: it is an object KEY into
// state.networkEndpoints. A corrupt "__proto__" would set that map's
// prototype instead of an own key, so the user's endpoint would silently
// not be recorded and a switch away and back would return the public
// default. isKnownNetworkId() is an own-property test against the network
// table for exactly that reason.
if (
has(saved, "networkId") &&
saved.networkId !== undefined &&
!isKnownNetworkId(saved.networkId)
) {
return (
"The saved data selects a network AutistMask does not know (" +
JSON.stringify(saved.networkId) +
")."
);
}
return null;
}
/**
* Refuse a record this build cannot use.
*
* @param {*} saved the raw record from storage.
* @throws {StateUnusableError}
*/
function assertStateUsable(saved) {
const problem = stateProblem(saved);
if (problem) throw new StateUnusableError(problem);
}
/**
* Whether `saved` is a usable record written before versions existed, and so
* gets the current version stamped on it the next time anything writes. Purely
* informational — the migration itself is that stamp, since version 1 IS the
* unversioned shape.
*
* @param {*} saved
* @returns {boolean}
*/
function migrationNeeded(saved) {
return (
isPlainObject(saved) &&
!has(saved, "schemaVersion") &&
stateProblem(saved) === null
);
}
module.exports = {
STATE_SCHEMA_VERSION,
StateUnusableError,
assertStateUsable,
migrationNeeded,
stateProblem,
};