From 76fb0e57a35042f9f56435c1524f104c1c0ccdca Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 5 Oct 2026 01:03:15 +0000 Subject: [PATCH] harden: lost-password confirmation refuses empty input and ignores invisible characters (closes #336) A wallet named only with spaces compared equal to an empty field, so typing nothing would have deleted it, and a zero-width space in a name made the name impossible to type back. An empty typed confirmation is now refused whatever the name is. The characters src/shared/symbolSpoof.js already defines as painting nothing are removed from both sides before comparing. A name that shows nothing at all is shown on the delete screens as "Wallet N", so it can still be typed back. Model: opus-5-5 --- README.md | 20 +++++--- TODO.md | 9 ++++ src/popup/views/deleteWallet.js | 39 ++++++++++----- tests/deleteWalletLostPassword.test.js | 69 ++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 2515733..b1802a2 100644 --- a/README.md +++ b/README.md @@ -1788,7 +1788,10 @@ view would leave a wallet one click from deletion. new password — and, in bold, that without that phrase written down the deletion loses everything the wallet holds, forever - That the other wallets are not touched - - The wallet's name, and a text input asking for it to be typed back + - The wallet's name, and a text input asking for it to be typed back. A name + that shows nothing at all (only spaces, or only characters that paint + nothing) is shown as "Wallet N", its position in the list, and that is + what is typed back. - Error line - "Delete This Wallet Forever" button - **Transitions**: @@ -1796,9 +1799,9 @@ view would leave a wallet one click from deletion. outcomes as "Confirm Delete" above, through the same `finishDelete()`, so the selection repair, permission cleanup and `AUTISTMASK_ACTIVE_CHANGED` broadcast are identical on both routes - - "Delete This Wallet Forever" (name does not match) → "That is not the name - of this wallet. Type <name> to confirm." on the error line, nothing - deleted + - "Delete This Wallet Forever" (name does not match, or the field is empty) + → "That is not the name of this wallet. Type <name> to confirm." on + the error line, nothing deleted - "Back" → **DeleteWallet**, re-entered through its `show()` so the wallet selection comes back with it. The two delete screens are siblings rather than parent and child: nothing is pushed on the way here, so both have @@ -1807,8 +1810,13 @@ view would leave a wallet one click from deletion. secret protects nobody: an attacker at the popup who wants the wallet gone can uninstall the extension, so the only person such a gate stops is the owner who forgot it. The typed name is a check that the user knows which wallet they are - on, not a secret, so it is matched with surrounding spaces and letter case - ignored. + on, not a secret, so it is matched as the user can see it: letter case, + surrounding spaces and repeated inner spaces are ignored, and characters that + paint nothing (format characters such as the zero-width space, + default-ignorable characters, and DELETE — the same set + `src/shared/symbolSpoof.js` strips) are removed from both sides before + comparing. An empty field, or one holding only spaces or such characters, is + refused whatever the wallet is called. - Not in `RESTORABLE_VIEWS`, alongside `delete-wallet-confirm`: a popup reopened by accident must not land on a screen whose button erases key material. diff --git a/TODO.md b/TODO.md index fa4acd0..451eadb 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-05: The lost-password delete confirmation refuses an empty field and + ignores characters that paint nothing + ([#336](https://git.eeqj.de/sneak/AutistMask/issues/336)). A wallet named only + with spaces compared equal to an empty field, so typing nothing would have + deleted it, and a zero-width space in a name made the name impossible to type + back. An empty field is now refused whatever the name is, the same invisible + characters `src/shared/symbolSpoof.js` strips are removed from both sides, and + a name that shows nothing is shown and typed back as "Wallet N". + - 2026-10-05: A `holders_count` that is not a whole number in plain digits is unknown, not read in part ([#251](https://git.eeqj.de/sneak/AutistMask/issues/251)). `parseInt` read diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js index 896d7fe..a1e284d 100644 --- a/src/popup/views/deleteWallet.js +++ b/src/popup/views/deleteWallet.js @@ -12,6 +12,7 @@ const { removeWalletFromState, broadcastActiveChanged, } = require("../../shared/walletDelete"); +const { INVISIBLE_CHARACTERS } = require("../../shared/symbolSpoof"); let deleteWalletIndex = null; let lostPasswordIndex = null; @@ -20,21 +21,31 @@ let ctx = null; // The name shown for a wallet, and on the lost-password screen the string // the user has to type back. One function so the two cannot disagree: a // confirmation that asks for a name other than the one on screen is -// unusable. +// unusable. A name that shows nothing at all (only spaces, or only +// zero-width characters) is replaced by "Wallet N" for the same reason: +// there would be nothing on screen to type back. function displayName(walletIdx) { const wallet = state.wallets[walletIdx]; - return (wallet && wallet.name) || "Wallet " + (walletIdx + 1); + const name = wallet && wallet.name; + if (name && confirmKey(name)) return name; + return "Wallet " + (walletIdx + 1); } // What the typed confirmation and the wallet name are compared as. HTML // collapses runs of whitespace when it renders the name, so a wallet named // "My Wallet" with two spaces DISPLAYS as "My Wallet": the user cannot // see the second space and cannot type a string that matches the stored -// name. Comparing collapsed on both sides is what keeps the confirmation -// satisfiable, on the one screen whose whole purpose is unwedging a user -// who is already stuck. Case and surrounding space go the same way. +// name. Characters that paint nothing, such as a zero-width space, are +// invisible the same way and are removed first. Comparing this form on +// both sides is what keeps the confirmation satisfiable, on the one screen +// whose whole purpose is unwedging a user who is already stuck. Case and +// surrounding space go the same way. function confirmKey(name) { - return name.trim().replace(/\s+/g, " ").toLowerCase(); + return name + .replace(INVISIBLE_CHARACTERS, "") + .trim() + .replace(/\s+/g, " ") + .toLowerCase(); } // Drop the password from the DOM and the wallet selection from the @@ -174,14 +185,16 @@ function init(_ctx) { return; } - // Case, surrounding spaces and repeated inner spaces are not part - // of the confirmation; see confirmKey(). This asks whether the - // user knows which wallet they are on; it is not a secret, and - // refusing "wallet 2" for "Wallet 2" would only teach the user to - // distrust the control. - const typed = $("delete-wallet-lost-name-input").value; + // Case, surrounding spaces, repeated inner spaces and invisible + // characters are not part of the confirmation; see confirmKey(). + // This asks whether the user knows which wallet they are on; it is + // not a secret, and refusing "wallet 2" for "Wallet 2" would only + // teach the user to distrust the control. An empty field is + // refused whatever the wallet is called, so no stored name can + // ever be confirmed by typing nothing. + const typed = confirmKey($("delete-wallet-lost-name-input").value); const expected = displayName(lostPasswordIndex); - if (confirmKey(typed) !== confirmKey(expected)) { + if (typed === "" || typed !== confirmKey(expected)) { $("delete-wallet-lost-flash").textContent = "That is not the name of this wallet. Type " + expected + diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js index aa24e8c..b28c3fe 100644 --- a/tests/deleteWalletLostPassword.test.js +++ b/tests/deleteWalletLostPassword.test.js @@ -46,6 +46,9 @@ const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599"; const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; +// U+200B, built from its code point so that it can be seen in this file. +const ZERO_WIDTH_SPACE = String.fromCodePoint(0x200b); + // ------------------------------------------------------------ DOM stub function makeElement(id) { @@ -342,6 +345,72 @@ describe("the typed confirmation", () => { "secret-three", ]); }); + + // A name of only spaces compares as nothing, and so does an empty + // field. Typing nothing must still delete nothing. + test.each(["", " "])( + "typing %j deletes nothing when the name is only spaces", + async (typedValue) => { + const { deleteWallet, state, storage } = load(); + state.wallets[1].name = " "; + await openLostPassword(deleteWallet, 1); + + node("delete-wallet-lost-name-input").value = typedValue; + await click("btn-delete-wallet-lost-confirm"); + + expect(node("delete-wallet-lost-flash").style.visibility).toBe( + "visible", + ); + expect(state.wallets).toHaveLength(3); + expect(await persistedWallets(storage)).toHaveLength(3); + }, + ); + + // A name that shows nothing would leave nothing on screen to type + // back, so the screen names the wallet by its position instead, and + // that is what the user types. + test.each([ + ["spaces", " "], + ["a zero-width space", ZERO_WIDTH_SPACE], + ])( + "a name of only %s is shown and typed back as Wallet 2", + async (_label, storedName) => { + const { deleteWallet, state, storage } = load(); + state.wallets[1].name = storedName; + await openLostPassword(deleteWallet, 1); + + expect(node("delete-wallet-lost-name").textContent).toBe( + "Wallet 2", + ); + node("delete-wallet-lost-name-input").value = "Wallet 2"; + await click("btn-delete-wallet-lost-confirm"); + + const persisted = await persistedWallets(storage); + expect(persisted.map((w) => w.encryptedSecret)).toEqual([ + "secret-one", + "secret-three", + ]); + }, + ); + + // A zero-width space paints nothing, so "My", a zero-width space and + // "Wallet" reads as "MyWallet", and that is all the user can type. HTML + // does not collapse it the way it collapses spaces, so it has to be + // removed explicitly. + test("a zero-width space inside the name is not part of it", async () => { + const { deleteWallet, state, storage } = load(); + state.wallets[1].name = "My" + ZERO_WIDTH_SPACE + "Wallet"; + await openLostPassword(deleteWallet, 1); + + node("delete-wallet-lost-name-input").value = "MyWallet"; + await click("btn-delete-wallet-lost-confirm"); + + const persisted = await persistedWallets(storage); + expect(persisted.map((w) => w.encryptedSecret)).toEqual([ + "secret-one", + "secret-three", + ]); + }); }); describe("deleting without the password", () => {