fix: malformed allowedSites and fraudContracts entries are dereferenced without a floor #362

New Issue

2026-08-23T19:02:23+02:00

clawbot commented

2026-08-23 19:02:23 +02:00

Found by the re-review of #360 (#360 (comment)), and a sibling case found during its rework. Both pre-existing on next, both deliberately left out of that PR's scope.

1. `allowedSites` — a working popup that silently never persists again

A stored allowedSites whose value is not a list — e.g. {"0xabc…": "notalist"} — passes the state gate and renders a working popup, then throws an unhandled TypeError: base.map is not a function at src/shared/state.js:217.

The failure mode is the nastiest shape available: the UI looks entirely healthy, and every subsequent save silently fails. The user keeps using the wallet while nothing they do is persisted. That is worse than the blank popup #311 fixed, because a blank popup is at least visibly broken.

Reproduced independently twice — by the reviewer and again during rework, where allowedSites: {a: 1} killed the jest worker as an unhandled rejection after the popup had rendered.

2. `fraudContracts` — same class, milder consequence

fraudContracts is assigned as saved.x || default with no type check, and is then structurally dereferenced at src/popup/views/send.js:124: (state.fraudContracts || []).map((a) => a.toLowerCase()).

It does not blank the popup — the boot path reaches it only via loadHomeTxs(), which catches and renders "Failed to load transactions", and the send view is not on boot. So the consequence is a broken send screen rather than silent data loss, but the shape is identical: a container floored (or not) while its entries are dereferenced unchecked.

Why these are together

#360 established the idiom for this class — floor the container and its entries, dropping anything that cannot be safely dereferenced — and applied it to trackedTokens and tokenBalances. These are the two fields it did not reach. Fix them with that same idiom rather than inventing a third one.

In milestone 1.0.0 on the strength of case 1: silent non-persistence in a wallet is a data-loss defect, and the 1.0 standard is being able to actually use this in low-volume production.

Definition of done

Malformed allowedSites, deniedSites and fraudContracts entries cannot reach a .map() or a .toLowerCase(). Floor the per-entry shape, not just the container, following the idiom settled in #360.
A save that fails must not fail silently — the user is told, or the state is repaired, but the wallet must never continue running against storage that is quietly rejecting every write.
Test per field: a malformed entry yields a usable popup whose writes actually persist, and a usable send screen. Each fails against current head — state the mutation and observed result.
make check green.

Found by the re-review of https://git.eeqj.de/sneak/AutistMask/pulls/360 (https://git.eeqj.de/sneak/AutistMask/pulls/360#issuecomment-69234), and a sibling case found during its rework. Both pre-existing on `next`, both deliberately left out of that PR's scope. ## 1. `allowedSites` — a working popup that silently never persists again A stored `allowedSites` whose value is not a list — e.g. `{"0xabc…": "notalist"}` — passes the state gate and renders a **working** popup, then throws an unhandled `TypeError: base.map is not a function` at `src/shared/state.js:217`. The failure mode is the nastiest shape available: the UI looks entirely healthy, and **every subsequent save silently fails**. The user keeps using the wallet while nothing they do is persisted. That is worse than the blank popup https://git.eeqj.de/sneak/AutistMask/issues/311 fixed, because a blank popup is at least visibly broken. Reproduced independently twice — by the reviewer and again during rework, where `allowedSites: {a: 1}` killed the jest worker as an unhandled rejection *after* the popup had rendered. ## 2. `fraudContracts` — same class, milder consequence `fraudContracts` is assigned as `saved.x || default` with no type check, and is then structurally dereferenced at `src/popup/views/send.js:124`: `(state.fraudContracts || []).map((a) => a.toLowerCase())`. It does not blank the popup — the boot path reaches it only via `loadHomeTxs()`, which catches and renders "Failed to load transactions", and the send view is not on boot. So the consequence is a broken send screen rather than silent data loss, but the shape is identical: a container floored (or not) while its entries are dereferenced unchecked. ## Why these are together https://git.eeqj.de/sneak/AutistMask/pulls/360 established the idiom for this class — floor the container **and** its entries, dropping anything that cannot be safely dereferenced — and applied it to `trackedTokens` and `tokenBalances`. These are the two fields it did not reach. Fix them with that same idiom rather than inventing a third one. In milestone 1.0.0 on the strength of case 1: silent non-persistence in a wallet is a data-loss defect, and the 1.0 standard is being able to actually use this in low-volume production. ## Definition of done - [ ] Malformed `allowedSites`, `deniedSites` and `fraudContracts` entries cannot reach a `.map()` or a `.toLowerCase()`. Floor the per-entry shape, not just the container, following the idiom settled in https://git.eeqj.de/sneak/AutistMask/pulls/360. - [ ] A save that fails must not fail silently — the user is told, or the state is repaired, but the wallet must never continue running against storage that is quietly rejecting every write. - [ ] Test per field: a malformed entry yields a usable popup whose writes actually persist, and a usable send screen. Each fails against current head — state the mutation and observed result. - [ ] `make check` green.

clawbot added this to the 1.0.0 milestone 2026-08-23 19:02:23 +02:00

clawbot changed title from ~~fix: a malformed allowedSites entry gives a working popup that silently never persists again~~ to fix: malformed allowedSites and fraudContracts entries are dereferenced without a floor

2026-08-23 19:21:22 +02:00

clawbot referenced this issue

2026-08-23 20:02:57 +02:00

fix: version the stored profile, and give a record that cannot be read a way out (closes #311) #360

clawbot referenced this issue from a commit

2026-08-23 20:22:11 +02:00

fix: floor the entries of the site maps and the fraud list, and stop a failed save from failing silently (closes #362)

clawbot referenced a pull request that will close this issue

2026-08-23 20:22:56 +02:00

fix: floor the persisted fields a restore dereferences, and make each field's floor an executable claim (closes #362) #366

clawbot referenced this issue from a commit

2026-08-23 21:00:29 +02:00