fix: stored state has no version and no migration, and a corrupt blob bricks the popup and every dApp call #311

New Issue

clawbot · 2026-08-20T12:00:00+02:00

clawbot commented

2026-08-20 12:00:00 +02:00

Found by the pre-1.0 deployability audit (#303). Blocker: no upgrade story, and no in-product way out of a bad blob.

saveState() (src/shared/state.js:83-113) enumerates 28 fields. Read back from a live install, there is no version or schemaVersion among them:

[PROBE] A2: stored keys = activeAddress,allowedSites,blockscoutUrl,currentView,debugMode,
        deniedSites,dustThresholdGwei,fraudContracts,hasWallet,hideDustTransactions,
        hideFraudContracts,hideLowHolderTokens,hideSpoofedSymbols,lastBalanceRefresh,networkId,
        rememberSiteChoice,rpcUrl,selectedAddress,selectedToken,selectedWallet,
        showZeroBalanceTokens,theme,tokenHolderCache,trackedTokens,utcTimestamps,viewData,
        viewStack,wallets
[PROBE] A2: is there a version/schema field? false

loadState() coerces scalars but validates nothing structural.

Reproduction

Hostile blobs written to storage, then the real popup opened:

--- case: wallets is a string
    visibleViews: []   bodyText: "AutistMask by @sneak"
    errors: ["pageerror: Cannot read properties of undefined (reading 'length')"]
--- case: wallets is an array of garbage
    visibleViews: []   bodyText: "AutistMask by @sneak"
    errors: ["pageerror: Cannot read properties of null (reading 'addresses')"]
--- case: future-schema blob (unknown fields, no version)
    visibleViews: []   bodyText: "AutistMask by @sneak"
    errors: ["pageerror: Cannot read properties of undefined (reading 'length')"]

A completely blank popup: no view visible, no message, no recovery control.

It is not only the UI. With such a blob in storage, getActiveAddress() (src/background/index.js:191-199) dereferences s.wallets[0].addresses[0] and throws, so every dApp RPC call from every page answers -32603 AutistMask could not complete this request because of an internal error. And there is no reset or wipe control anywhere in the UI — src/popup/ and src/background/ were grepped for one.

Consequence

An unversioned blob plus any future schema change is a bricked wallet on auto-update, with no way out from inside the product. Today the same state is reachable from a partial write or a downgrade. The version field is the cheap insurance that is missing.

Note the adjacent latent case: networkById() (src/shared/networks.js:34-36) returns mainnet for any unknown id — a stored {networkId:"base"} renders the selector as mainnet with no banner and eth_chainId 0x1, while rpcUrl still points at Base. Unreachable through today's UI (two networks, no free-form field), but it is exactly the failure mode a downgrade would produce.

Definition of done

saveState() stamps a schema version.
loadState() validates the shape. On a version it does not understand, or a wallets array it cannot parse, it refuses to run and shows an explicit screen naming the problem, rather than rendering blank.
That screen offers an export of the raw blob and an explicit destructive reset behind a typed confirmation, so the user is never trapped.
The background fails the same way: a dApp call against unusable state returns a specific error, not a generic -32603.
networkById() on an unknown id fails loudly rather than silently returning mainnet.
Test: each of the three corrupt blobs above yields the recovery screen, not a blank popup.
make check green.

Found by the pre-1.0 deployability audit (https://git.eeqj.de/sneak/AutistMask/issues/303). **Blocker: no upgrade story, and no in-product way out of a bad blob.** `saveState()` (`src/shared/state.js:83-113`) enumerates 28 fields. Read back from a live install, there is **no `version` or `schemaVersion` among them**: ``` [PROBE] A2: stored keys = activeAddress,allowedSites,blockscoutUrl,currentView,debugMode, deniedSites,dustThresholdGwei,fraudContracts,hasWallet,hideDustTransactions, hideFraudContracts,hideLowHolderTokens,hideSpoofedSymbols,lastBalanceRefresh,networkId, rememberSiteChoice,rpcUrl,selectedAddress,selectedToken,selectedWallet, showZeroBalanceTokens,theme,tokenHolderCache,trackedTokens,utcTimestamps,viewData, viewStack,wallets [PROBE] A2: is there a version/schema field? false ``` `loadState()` coerces scalars but validates nothing structural. ## Reproduction Hostile blobs written to storage, then the real popup opened: ``` --- case: wallets is a string visibleViews: [] bodyText: "AutistMask by @sneak" errors: ["pageerror: Cannot read properties of undefined (reading 'length')"] --- case: wallets is an array of garbage visibleViews: [] bodyText: "AutistMask by @sneak" errors: ["pageerror: Cannot read properties of null (reading 'addresses')"] --- case: future-schema blob (unknown fields, no version) visibleViews: [] bodyText: "AutistMask by @sneak" errors: ["pageerror: Cannot read properties of undefined (reading 'length')"] ``` A completely blank popup: no view visible, no message, no recovery control. It is not only the UI. With such a blob in storage, `getActiveAddress()` (`src/background/index.js:191-199`) dereferences `s.wallets[0].addresses[0]` and throws, so **every dApp RPC call from every page** answers `-32603 AutistMask could not complete this request because of an internal error`. And there is **no reset or wipe control anywhere in the UI** — `src/popup/` and `src/background/` were grepped for one. ## Consequence An unversioned blob plus any future schema change is a bricked wallet on auto-update, with no way out from inside the product. Today the same state is reachable from a partial write or a downgrade. The version field is the cheap insurance that is missing. Note the adjacent latent case: `networkById()` (`src/shared/networks.js:34-36`) returns **mainnet** for any unknown id — a stored `{networkId:"base"}` renders the selector as `mainnet` with no banner and `eth_chainId` `0x1`, while `rpcUrl` still points at Base. Unreachable through today's UI (two networks, no free-form field), but it is exactly the failure mode a downgrade would produce. ## Definition of done - [ ] `saveState()` stamps a schema version. - [ ] `loadState()` validates the shape. On a version it does not understand, or a `wallets` array it cannot parse, it **refuses to run** and shows an explicit screen naming the problem, rather than rendering blank. - [ ] That screen offers an export of the raw blob **and** an explicit destructive reset behind a typed confirmation, so the user is never trapped. - [ ] The background fails the same way: a dApp call against unusable state returns a specific error, not a generic `-32603`. - [ ] `networkById()` on an unknown id fails loudly rather than silently returning mainnet. - [ ] Test: each of the three corrupt blobs above yields the recovery screen, not a blank popup. - [ ] `make check` green.

clawbot added this to the 1.0.0 milestone 2026-08-20 12:00:00 +02:00

clawbot referenced this issue

2026-08-20 12:01:08 +02:00

pre-1.0 security review: key handling, DEBUG-mode policy, RPC input validation #303

clawbot commented

2026-08-20 12:37:21 +02:00

Trap for whoever implements this, introduced by #313: state.networkId is loaded unvalidated (src/shared/state.js:136) and is now used as an object key into the new state.networkEndpoints map. A corrupt networkId of "__proto__" sets the map's prototype instead of an own key, so the user's endpoint is silently not recorded and a switch away and back returns the public default.

Not reachable today — only this codebase writes that field, and only with validated network ids — which is why it is a note here rather than its own issue. It becomes reachable the moment stored state is treated as untrusted, which is exactly what this issue asks for. Validate networkId against src/shared/networks.js as part of the shape validation.

Trap for whoever implements this, introduced by https://git.eeqj.de/sneak/AutistMask/pulls/313: `state.networkId` is loaded unvalidated (`src/shared/state.js:136`) and is now used as an **object key** into the new `state.networkEndpoints` map. A corrupt `networkId` of `"__proto__"` sets the map's prototype instead of an own key, so the user's endpoint is silently not recorded and a switch away and back returns the public default. Not reachable today — only this codebase writes that field, and only with validated network ids — which is why it is a note here rather than its own issue. It becomes reachable the moment stored state is treated as untrusted, which is exactly what this issue asks for. Validate `networkId` against `src/shared/networks.js` as part of the shape validation.

clawbot referenced this issue

2026-08-20 12:49:01 +02:00

fix: a cold worker prepares a dApp transaction for the wrong chain, so every non-mainnet dApp send is refused #320

clawbot referenced this issue

2026-08-20 12:57:27 +02:00

harden: the background must stop reading the shared state singleton — one root cause, five sites, and the fixes keep adding new ones #324

clawbot referenced this issue

2026-08-20 14:26:18 +02:00

pre-1.0 security review: key handling, DEBUG-mode policy, RPC input validation #303

Sign in to join this conversation.