sneak/AutistMask
1
0
Fork 0
Code Issues 60 Pull Requests Actions Packages Projects Releases Wiki Activity

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

New Issue
Closed
opened 2026-08-20 12:00:00 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-20 12:00:00 +02:00
Collaborator
Copy Link

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 commented 2026-08-20 12:37:21 +02:00
Author
Collaborator
Copy Link

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 commented 2026-08-23 18:05:36 +02:00
Author
Collaborator
Copy Link

Implementation plan, before code.

Version and migration. STATE_SCHEMA_VERSION = 1 in a new src/shared/stateSchema.js. Every write path stamps it: the popup's saveState() and the background's updateState(). An UNVERSIONED but structurally valid blob — which is what every install in the field has — is treated as version 1 and migrated in place on the next write. It never reaches the recovery screen; that case gets its own test, because a wipe prompt for a perfectly good wallet on upgrade is the worse failure.

Validation. assertStateUsable(raw) runs on the raw record before normalization, on both read paths (loadState() in src/shared/state.js, getState() in src/background/state.js), and throws StateUnusableError carrying a user-facing sentence naming the problem. It refuses: a record that is not an object; a schemaVersion that is not an integer this build understands (newer blob included); wallets that is not an array, or whose entries are not wallet records with an addresses array of address records; and a networkId that is not a network in src/shared/networks.js. Every key test is Object.prototype.hasOwnProperty, so "__proto__" and "constructor" are refused rather than resolving through the prototype chain — the trap in the comment above.

Popup. init() catches StateUnusableError and shows a new state-recovery screen instead of proceeding; nothing else runs, and the Settings gear is hidden, because every other screen reads the profile. The screen names the problem, offers an export of the raw blob (revealed in a selectable textarea, which always works, plus a best-effort file download), and a destructive reset behind a typed confirmation. showView() is not used on this path: it reads and writes the state singleton, which by then refuses to be read.

Background. getState() throws the same error, and the RPC dispatcher answers a specific code and message — the wallet cannot read its saved data, nothing was signed or sent, open the extension — instead of the generic -32603.

networkById. Throws UnknownNetworkError on an unknown id instead of returning mainnet, matching getProvider(). Also fixes networkById("constructor") currently answering with Object.

Tests. The three corrupt blobs from the issue drive the real popup entry point over a DOM stub and must land on the recovery screen; the unversioned-but-valid blob must load, keep its wallet, and gain the version stamp. The dApp path gets the specific error over the existing cold-worker harness. Fail-first output for all four goes in the PR body.

Implementation plan, before code. **Version and migration.** `STATE_SCHEMA_VERSION = 1` in a new `src/shared/stateSchema.js`. Every write path stamps it: the popup's `saveState()` and the background's `updateState()`. An UNVERSIONED but structurally valid blob — which is what every install in the field has — is treated as version 1 and migrated in place on the next write. It never reaches the recovery screen; that case gets its own test, because a wipe prompt for a perfectly good wallet on upgrade is the worse failure. **Validation.** `assertStateUsable(raw)` runs on the raw record before normalization, on both read paths (`loadState()` in `src/shared/state.js`, `getState()` in `src/background/state.js`), and throws `StateUnusableError` carrying a user-facing sentence naming the problem. It refuses: a record that is not an object; a `schemaVersion` that is not an integer this build understands (newer blob included); `wallets` that is not an array, or whose entries are not wallet records with an `addresses` array of address records; and a `networkId` that is not a network in `src/shared/networks.js`. Every key test is `Object.prototype.hasOwnProperty`, so `"__proto__"` and `"constructor"` are refused rather than resolving through the prototype chain — the trap in the comment above. **Popup.** `init()` catches `StateUnusableError` and shows a new `state-recovery` screen instead of proceeding; nothing else runs, and the Settings gear is hidden, because every other screen reads the profile. The screen names the problem, offers an export of the raw blob (revealed in a selectable textarea, which always works, plus a best-effort file download), and a destructive reset behind a typed confirmation. `showView()` is not used on this path: it reads and writes the state singleton, which by then refuses to be read. **Background.** `getState()` throws the same error, and the RPC dispatcher answers a specific code and message — the wallet cannot read its saved data, nothing was signed or sent, open the extension — instead of the generic `-32603`. **networkById.** Throws `UnknownNetworkError` on an unknown id instead of returning mainnet, matching `getProvider()`. Also fixes `networkById("constructor")` currently answering with `Object`. **Tests.** The three corrupt blobs from the issue drive the real popup entry point over a DOM stub and must land on the recovery screen; the unversioned-but-valid blob must load, keep its wallet, and gain the version stamp. The dApp path gets the specific error over the existing cold-worker harness. Fail-first output for all four goes in the PR body.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#311
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?