security: pre-1.0 review of key handling, DEBUG mode and RPC/dApp input validation #383

New Issue

2026-09-21T09:19:11+02:00

clawbot commented

2026-09-21 09:19:11 +02:00

TODO.md names this as the repo's Next Step and no issue exists for it: "Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC input validation) before any 1.0rc tag. Individual filed issues are parts of it, but the review is broader than any of them."

This is the one remaining piece of work between the current next and a version that can honestly be called 1.0.0. The milestone 1.0.0 PR (#190) has landed on main; that work was driven issue by issue and nobody has yet read the whole extension end to end with an attacker's eye.

Scope

An independent review session reads the whole tree on next and reports on, at minimum:

Key handling: src/shared/vault.js, src/shared/wallet.js, every path that decrypts, derives, displays or exports key material, what clears it and when, and where a secret could reach a log, a message, storage, or the DOM unexpectedly.
DEBUG mode: every DEBUG conditional in src/ against the policy in README.md and RULES.md (banner and test mnemonic only, no skipped functionality, no bypassed security), and that a release build cannot be made to behave as a debug one.
RPC and dApp input validation: everything src/background/index.js accepts from a page through src/content/inpage.js and src/content/index.js (method names, params, chain ids, addresses, typed data), everything it accepts from the configured RPC endpoint and the explorer, and everything the approval screens render from those inputs.
The permission model: site connection, origin checks, what a connected site can do without a prompt, and what a page can learn about the wallet without being connected.
Storage: what is persisted, what is encrypted, and whether a hostile stored record can still reach a signing path.

Definition of done

A review summary is posted on this issue: what was examined, what was found, and the boundary of what was not examined.
Every finding is filed as its own Gitea issue with a severity, a reproduction, and a definition of done; a finding that puts funds or key material at risk is added to milestone 1.0.0.
Findings are fixed and reviewed through the normal PR workflow; this issue closes when no finding rated funds-or-keys-at-risk remains open.
The result of the review is recorded in README.md or TODO.md at the width it actually holds, not wider.

model: claude-fable-5

`TODO.md` names this as the repo's Next Step and no issue exists for it: "Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC input validation) before any 1.0rc tag. Individual filed issues are parts of it, but the review is broader than any of them." This is the one remaining piece of work between the current `next` and a version that can honestly be called 1.0.0. The milestone 1.0.0 PR (https://git.eeqj.de/sneak/AutistMask/pulls/190) has landed on `main`; that work was driven issue by issue and nobody has yet read the whole extension end to end with an attacker's eye. ## Scope An independent review session reads the whole tree on `next` and reports on, at minimum: - Key handling: `src/shared/vault.js`, `src/shared/wallet.js`, every path that decrypts, derives, displays or exports key material, what clears it and when, and where a secret could reach a log, a message, storage, or the DOM unexpectedly. - DEBUG mode: every `DEBUG` conditional in `src/` against the policy in `README.md` and `RULES.md` (banner and test mnemonic only, no skipped functionality, no bypassed security), and that a release build cannot be made to behave as a debug one. - RPC and dApp input validation: everything `src/background/index.js` accepts from a page through `src/content/inpage.js` and `src/content/index.js` (method names, params, chain ids, addresses, typed data), everything it accepts from the configured RPC endpoint and the explorer, and everything the approval screens render from those inputs. - The permission model: site connection, origin checks, what a connected site can do without a prompt, and what a page can learn about the wallet without being connected. - Storage: what is persisted, what is encrypted, and whether a hostile stored record can still reach a signing path. ## Definition of done - [ ] A review summary is posted on this issue: what was examined, what was found, and the boundary of what was not examined. - [ ] Every finding is filed as its own Gitea issue with a severity, a reproduction, and a definition of done; a finding that puts funds or key material at risk is added to milestone 1.0.0. - [ ] Findings are fixed and reviewed through the normal PR workflow; this issue closes when no finding rated funds-or-keys-at-risk remains open. - [ ] The result of the review is recorded in `README.md` or `TODO.md` at the width it actually holds, not wider. model: claude-fable-5

clawbot added this to the 1.0.0 milestone 2026-09-21 09:19:11 +02:00

clawbot referenced this issue

2026-09-21 09:19:58 +02:00

Review toward 1.0: identify and file all next steps #382

clawbot referenced this issue

2026-09-21 18:32:21 +02:00

Review toward 1.0: identify and file all next steps #382

clawbot commented

2026-09-21 20:39:01 +02:00

Pre-1.0 security review — summary

Reviewed the whole tree at 99292b9 with an attacker's eye, line by line:
key handling (vault.js, wallet.js, and every decrypt/derive/display/export
path), the DEBUG build boundary, everything the background accepts from a page
and from the configured RPC endpoint and explorer, the approval and verification
path, what the approval screens render, the permission model, and storage.

Found sound and not filed: the background populates every dApp transaction
itself, displays that object, and verifies the signed artifact against it field
by field, refusing transaction types other than 0/1/2 and any
authorizationList/blob fields, and requiring the artifact to be the canonical
re-encoding of the approved transaction; secrets are decrypted only in the
popup, never cross the message boundary, and are wiped on view-leave under a
generation guard; the stored-record gate refuses an unusable profile before
normalization and uses own-property tests against prototype keys; extended keys
are round-trip checked on import; the CSP is pinned in both directions; there is
no externally_connectable; and isExtensionSender fails closed. Connecting an
opaque or non-http origin was considered and not filed: content scripts do not
run in sandboxed subframes (all_frames defaults to false) and file:// access
is off by default, so the path is not reachable in a default install.

Findings filed

Funds or keys at risk (milestone 1.0.0):

#399 — the fee backstop is checked
per field, so a page (or a hostile RPC) can produce a max fee of thousands of
ETH, and the wallet's own send path applies no fee ceiling at all.

Privacy:

#398 — the EIP-6963 provider UUID is
persisted and re-announced to every page, so any site can read a stable
per-install cross-site tracking identifier (milestone 1.0.0).
#410 — runtime debug mode logs full
request URLs and bodies, which can include an RPC credential.

Hardening:

#400 — typed-data signing shows no
warning for allowance shapes (Permit/Permit2) and displays a primaryType it
does not enforce (milestone 1.0.0).
#402 — remembered site permissions
are keyed by hostname only, dropping the scheme and port.
#403 — the personal_sign display
decodes only hex and shows control and bidirectional characters verbatim,
without the raw bytes.
#404 — a page-supplied transaction
nonce is honoured and shown only as a bare number.
#405 — a page can open unbounded
concurrent approval windows.
#406 — a session (non-remembered)
connection cannot be revoked from the UI, and the remove-site message does
nothing.
#407 — origin attribution falls back
to the top-level tab URL and then to the page-supplied origin.

Decisions (assigned to sneak):

#401 — Argon2id runs at INTERACTIVE
cost for the at-rest vault key (milestone 1.0.0).
#408 — a connected site switches the
wallet's network with no prompt.
#409 — eth_sign is signed as a
personal message while its warning and the README describe raw-hash signing.

Not examined

The two end-to-end suites were not run (per the review brief). The vendored
phishing blocklist and the bundled token-list data were not audited entry by
entry — only the code that consults them. ethers and
libsodium-wrappers-sumo are trusted as audited third parties. No live-network
or real-funds pass was done; that is
#385.

Disclosures

The planned independent finder-and-adversarial-verification pass did not
complete: 17 of 18 agents stopped at this account's model limit, so the merge,
verification and completeness-critic stages did not run. These findings rest
on the reviewer's direct line-by-line reading of the tree (and one completed
finder), each re-checked against the code before filing, not on the full
skeptic-panel verification the method intends.
Reviewed at 99292b9; origin/next has since advanced to 9ac7df0 (only
#351), which touches none of these
findings.

Model: fable-5-1 (review); opus-4-8 (filing)

## Pre-1.0 security review — summary Reviewed the whole tree at `99292b9` with an attacker's eye, line by line: key handling (`vault.js`, `wallet.js`, and every decrypt/derive/display/export path), the DEBUG build boundary, everything the background accepts from a page and from the configured RPC endpoint and explorer, the approval and verification path, what the approval screens render, the permission model, and storage. Found sound and not filed: the background populates every dApp transaction itself, displays that object, and verifies the signed artifact against it field by field, refusing transaction types other than 0/1/2 and any `authorizationList`/blob fields, and requiring the artifact to be the canonical re-encoding of the approved transaction; secrets are decrypted only in the popup, never cross the message boundary, and are wiped on view-leave under a generation guard; the stored-record gate refuses an unusable profile before normalization and uses own-property tests against prototype keys; extended keys are round-trip checked on import; the CSP is pinned in both directions; there is no `externally_connectable`; and `isExtensionSender` fails closed. Connecting an opaque or non-http origin was considered and not filed: content scripts do not run in sandboxed subframes (`all_frames` defaults to false) and `file://` access is off by default, so the path is not reachable in a default install. ### Findings filed Funds or keys at risk (milestone 1.0.0): - https://git.eeqj.de/sneak/AutistMask/issues/399 — the fee backstop is checked per field, so a page (or a hostile RPC) can produce a max fee of thousands of ETH, and the wallet's own send path applies no fee ceiling at all. Privacy: - https://git.eeqj.de/sneak/AutistMask/issues/398 — the EIP-6963 provider UUID is persisted and re-announced to every page, so any site can read a stable per-install cross-site tracking identifier (milestone 1.0.0). - https://git.eeqj.de/sneak/AutistMask/issues/410 — runtime debug mode logs full request URLs and bodies, which can include an RPC credential. Hardening: - https://git.eeqj.de/sneak/AutistMask/issues/400 — typed-data signing shows no warning for allowance shapes (Permit/Permit2) and displays a `primaryType` it does not enforce (milestone 1.0.0). - https://git.eeqj.de/sneak/AutistMask/issues/402 — remembered site permissions are keyed by hostname only, dropping the scheme and port. - https://git.eeqj.de/sneak/AutistMask/issues/403 — the personal_sign display decodes only hex and shows control and bidirectional characters verbatim, without the raw bytes. - https://git.eeqj.de/sneak/AutistMask/issues/404 — a page-supplied transaction nonce is honoured and shown only as a bare number. - https://git.eeqj.de/sneak/AutistMask/issues/405 — a page can open unbounded concurrent approval windows. - https://git.eeqj.de/sneak/AutistMask/issues/406 — a session (non-remembered) connection cannot be revoked from the UI, and the remove-site message does nothing. - https://git.eeqj.de/sneak/AutistMask/issues/407 — origin attribution falls back to the top-level tab URL and then to the page-supplied origin. Decisions (assigned to sneak): - https://git.eeqj.de/sneak/AutistMask/issues/401 — Argon2id runs at INTERACTIVE cost for the at-rest vault key (milestone 1.0.0). - https://git.eeqj.de/sneak/AutistMask/issues/408 — a connected site switches the wallet's network with no prompt. - https://git.eeqj.de/sneak/AutistMask/issues/409 — eth_sign is signed as a personal message while its warning and the README describe raw-hash signing. ### Not examined The two end-to-end suites were not run (per the review brief). The vendored phishing blocklist and the bundled token-list data were not audited entry by entry — only the code that consults them. `ethers` and `libsodium-wrappers-sumo` are trusted as audited third parties. No live-network or real-funds pass was done; that is https://git.eeqj.de/sneak/AutistMask/issues/385. ### Disclosures - The planned independent finder-and-adversarial-verification pass did not complete: 17 of 18 agents stopped at this account's model limit, so the merge, verification and completeness-critic stages did not run. These findings rest on the reviewer's direct line-by-line reading of the tree (and one completed finder), each re-checked against the code before filing, not on the full skeptic-panel verification the method intends. - Reviewed at `99292b9`; `origin/next` has since advanced to `9ac7df0` (only https://git.eeqj.de/sneak/AutistMask/issues/351), which touches none of these findings. Model: fable-5-1 (review); opus-4-8 (filing)

clawbot referenced this issue

2026-09-21 21:46:03 +02:00

milestone 1.0.0 release readiness: user-facing fixes, security review, release procedure #388

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#383