harden: two latent gaps in the lost-password confirmation — whitespace-only name, and U+200B still untypable #336

Open
opened 2026-08-20 15:12:42 +02:00 by clawbot · 0 comments
Collaborator

Found by the review of #334 (#334 (comment)). Both are unreachable in the product today. Filed because the first becomes live on a change elsewhere, and this confirmation guards a route that destroys a wallet with no password.

  1. A whitespace-only wallet name would accept an empty confirmation. confirmKey(" ") === "" === confirmKey(""), and there is no separate empty-input guard. Unreachable today because both rename paths (src/popup/views/settings.js:153, src/popup/views/home.js:360) do input.value.trim() and require a truthy result, and creation always names Wallet N. It becomes live the moment a wallet name is settable from anywhere that does not trim — an import path, a migration, a restored blob.
  2. U+200B is neither trimmed nor collapsed by \s, and HTML does not collapse it either, so a name containing a zero-width space remains untypable — the same class as the defect #312 fixed. Self-inflicted and escapable by renaming, unlike the original, which is why it is not a blocker.

Tab, newline and U+00A0 are all handled correctly, and name collision is harmless: the delete target is lostPasswordIndex, fixed when the screen opens (src/popup/views/deleteWallet.js:191), so the typed name is a check rather than a selector and two wallets sharing a confirmKey() cannot cause the wrong wallet to be destroyed.

Definition of done

  • An empty or whitespace-only typed confirmation is rejected outright, independently of what the name normalises to.
  • Zero-width characters are stripped or normalised on both sides, so no name can be untypable.
  • Tests for a whitespace-only name and a U+200B name; both fail against current head.
  • make check green.
Found by the review of https://git.eeqj.de/sneak/AutistMask/pulls/334 (https://git.eeqj.de/sneak/AutistMask/pulls/334#issuecomment-67650). **Both are unreachable in the product today.** Filed because the first becomes live on a change elsewhere, and this confirmation guards a route that destroys a wallet with no password. 1. **A whitespace-only wallet name would accept an empty confirmation.** `confirmKey(" ") === "" === confirmKey("")`, and there is no separate empty-input guard. Unreachable today because both rename paths (`src/popup/views/settings.js:153`, `src/popup/views/home.js:360`) do `input.value.trim()` and require a truthy result, and creation always names `Wallet N`. It becomes live the moment a wallet name is settable from anywhere that does not trim — an import path, a migration, a restored blob. 2. **U+200B is neither trimmed nor collapsed** by `\s`, and HTML does not collapse it either, so a name containing a zero-width space remains untypable — the same class as the defect #312 fixed. Self-inflicted and escapable by renaming, unlike the original, which is why it is not a blocker. Tab, newline and U+00A0 are all handled correctly, and name collision is harmless: the delete target is `lostPasswordIndex`, fixed when the screen opens (`src/popup/views/deleteWallet.js:191`), so the typed name is a check rather than a selector and two wallets sharing a `confirmKey()` cannot cause the wrong wallet to be destroyed. ## Definition of done - [ ] An empty or whitespace-only typed confirmation is rejected outright, independently of what the name normalises to. - [ ] Zero-width characters are stripped or normalised on both sides, so no name can be untypable. - [ ] Tests for a whitespace-only name and a U+200B name; both fail against current head. - [ ] `make check` green.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#336