From 4ba2e69599bc23f7ecedf42dcd281bfa77c7168c Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 16:52:46 +0000 Subject: [PATCH] harden: show a personal message's hex and its text in byte order, hidden characters marked (closes #403) The signature screen showed only the text a personal message decodes to, with bidirectional, right-to-left and zero-width characters acting on it, so a site could make the message read differently from the bytes that are signed, and a message that was not hex was decoded into NUL characters. The screen now shows the hex as "Raw data" alongside the text, lays the text out left to right in byte order, and shows each control character and each character that paints nothing (the set src/shared/symbolSpoof.js already strips) as a U+XXXX mark. A message is hex when getBytes, which signing uses, reads it; one that is not cannot be signed, so it is shown as plain text with "Sign" disabled. Model: opus-5-5 --- README.md | 23 +++- TODO.md | 11 ++ src/popup/index.html | 9 ++ src/popup/styles/main.css | 9 ++ src/popup/views/approval.js | 63 ++++++++-- src/shared/symbolSpoof.js | 8 +- tests/e2e/run.js | 41 +++++++ tests/personalSignDisplay.test.js | 195 ++++++++++++++++++++++++++++++ 8 files changed, 343 insertions(+), 16 deletions(-) create mode 100644 tests/personalSignDisplay.test.js diff --git a/README.md b/README.md index 9c4364e..5cad013 100644 --- a/README.md +++ b/README.md @@ -1927,10 +1927,17 @@ view would leave a wallet one click from deletion. - Danger warning box (shown for `eth_sign`, which signs a raw hash) - Type: "Personal message" or "Typed data (EIP-712)" - From: color dot + full address + etherscan link - - Message: decoded UTF-8 text (personal_sign) or formatted domain/type/ - message fields (EIP-712 typed data). The primary type shown is the one - ethers signs, derived from the typed data's `types`, not the type the site - states. + - Message: for `personal_sign` and `eth_sign`, the text the message's bytes + decode to as UTF-8, laid out left to right in the order of the bytes that + are signed, right-to-left characters included. Each control character, and + each character that paints nothing (format characters such as zero-width + and bidirectional ones, default-ignorable characters such as variation + selectors and Hangul fillers, and DELETE), is shown as a bordered `U+XXXX` + mark instead of acting on the text; a line feed is shown as a line break. + Bytes that are not UTF-8 are shown as "This message is not text." For + typed data, formatted domain/type/message fields (EIP-712). The primary + type shown is the one ethers signs, derived from the typed data's `types`, + not the type the site states. - Token permission warning, at the top of the message (typed data whose primary type is `Permit`, as in EIP-2612, or one of Permit2's signature types): "⚠️ TOKEN PERMISSION: Signing this lets the spender below take the @@ -1942,12 +1949,20 @@ view would leave a wallet one click from deletion. domain's `verifyingContract`; any those fields do not give is shown as `Unknown`, and the domain, type and message lines still follow. Only typed data that cannot be read at all is shown as raw text. + - Raw data (`personal_sign` and `eth_sign`): the message's hex exactly as + the site sent it. The bytes it encodes are what is signed, as an EIP-191 + personal message. - Password input and an error line - "Sign" / "Reject" buttons - **Transitions**: - Typed data that states no primary type, or one other than the type it would be signed as, or that cannot be read → shown with the error line saying so and "Sign" disabled; only "Reject" remains + - A `personal_sign` or `eth_sign` message that is not hex (`0x` or `0X` and + an even number of hex digits, the form ethers' `getBytes` reads when + signing) → shown as plain text, with the error line "This message is plain + text, not hex, so it cannot be signed." and "Sign" disabled; signing takes + the bytes from the hex, so such a message has none to sign - "Sign" (correct password) → signs locally → closes popup (returns signature) - "Sign" (wrong password, or a signing failure) → error line, no screen diff --git a/TODO.md b/TODO.md index ab2810c..a555ec4 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,17 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: The signature screen shows a personal message as the bytes that + are signed ([#403](https://git.eeqj.de/sneak/AutistMask/issues/403)). It + showed only the decoded text, with bidirectional and zero-width characters + acting on it, so a site could make the message read differently from what is + signed, and a message that was not hex was shown as NUL characters. The hex is + now shown as "Raw data" alongside the decoded text, the text is laid out left + to right in byte order, control characters and characters that paint nothing + are shown as `U+XXXX` marks, and a message that is not hex by the rule signing + reads it with is shown as plain text with "Sign" disabled, since such a + message has no bytes to sign. + - 2026-10-04: A site has at most one connection prompt and one signature prompt open at a time ([#405](https://git.eeqj.de/sneak/AutistMask/issues/405)). Each `eth_requestAccounts` or `personal_sign` call opened another approval window, diff --git a/src/popup/index.html b/src/popup/index.html index 2fd624d..208f185 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1698,6 +1698,15 @@ > + +
parseInt(b, 16)), - ); - return toUtf8String(bytes); + return toUtf8String(getBytes(hex)); } catch { return null; } } +// A character shown as a bordered U+XXXX mark. +function codePointMark(c) { + const code = c.codePointAt(0).toString(16).toUpperCase(); + return `U+${code.padStart(4, "0")}`; +} + +// The text as HTML, with each character that paints nothing (zero-width and +// bidirectional characters, variation selectors and Hangul fillers among +// them) and each control character shown as a mark. A line feed is shown as +// a line break. The marks are plain ASCII, so the second pass leaves them be. +function markInvisibleCharacters(text) { + return escapeHtml(text) + .replace(INVISIBLE_CHARACTERS, codePointMark) + .replace(/\p{Cc}/gu, (c) => (c === "\n" ? "
" : codePointMark(c))); +} + // The type ethers will sign typed data as. ethers does not read the page's // `primaryType`: it takes the one struct in `types` that no other struct // refers to. Throws when the types name no such single struct, which ethers @@ -657,15 +681,33 @@ function showSignApproval(details) { ? "Typed data (EIP-712)" : "Personal message"; + // A personal message is signed as the bytes its hex encodes, so the hex + // is shown as well as any text it decodes to, and that text is laid out + // left to right in the order of its bytes. Signing reads the bytes from + // the hex, so a message that is not hex cannot be signed: it is shown as + // the text it is, and refused. + let refusal = null; + $("approve-sign-hex-section").classList.add("hidden"); + $("approve-sign-message").classList.toggle("am-byte-order", !isTyped); if (isTyped) { $("approve-sign-message").innerHTML = formatTypedDataHtml(sp.typedData); - } else { + refusal = typedDataRefusal(sp); + } else if (isHexMessage(sp.message)) { const decoded = decodeHexMessage(sp.message); if (decoded !== null) { - $("approve-sign-message").textContent = decoded; + $("approve-sign-message").innerHTML = + markInvisibleCharacters(decoded); } else { - $("approve-sign-message").textContent = sp.message; + $("approve-sign-message").textContent = "This message is not text."; } + $("approve-sign-hex").textContent = sp.message; + $("approve-sign-hex-section").classList.remove("hidden"); + } else { + $("approve-sign-message").innerHTML = markInvisibleCharacters( + sp.message, + ); + refusal = + "This message is plain text, not hex, so it cannot be signed."; } // Display danger warning for eth_sign (raw hash signing) @@ -687,7 +729,6 @@ function showSignApproval(details) { showView("approve-sign"); attachCopyHandlers("view-approve-sign"); - const refusal = typedDataRefusal(sp); if (refusal) { showError("approve-sign-error", refusal); $("btn-approve-sign").disabled = true; diff --git a/src/shared/symbolSpoof.js b/src/shared/symbolSpoof.js index af908fe..5caea3d 100644 --- a/src/shared/symbolSpoof.js +++ b/src/shared/symbolSpoof.js @@ -34,6 +34,11 @@ function normalizeAddress(addr) { return (addr || "").toLowerCase(); } +// The characters that paint nothing; normalizeSymbol below says which they +// are. The signature screen marks them in a personal message +// (src/popup/views/approval.js). +const INVISIBLE_CHARACTERS = /[\p{Cf}\p{Default_Ignorable_Code_Point}\x7F]/gu; + // Fold a symbol onto what a user actually sees, and no further: // // NFKC collapses compatibility variants that render as the ASCII @@ -82,7 +87,7 @@ function normalizeAddress(addr) { function normalizeSymbol(symbol) { return String(symbol || "") .normalize("NFKC") - .replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}\x7F]/gu, "") + .replace(INVISIBLE_CHARACTERS, "") .trim() .toUpperCase(); } @@ -104,5 +109,6 @@ function isSpoofedSymbol(symbol, contractAddress) { } module.exports = { + INVISIBLE_CHARACTERS, isSpoofedSymbol, }; diff --git a/tests/e2e/run.js b/tests/e2e/run.js index e547926..cc34d03 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -3232,6 +3232,47 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env ); }); +// A right-to-left character must not move the characters around it: U+05C3 +// between "5" and "00" would otherwise put "500" on screen before it +// (https://git.eeqj.de/sneak/AutistMask/issues/403). +test("a personal message is laid out in the order of its bytes (#403)", async (env) => { + const text = "Pay 5" + String.fromCodePoint(0x05c3) + "00 ETH"; + await startRequest(env.dapp, "sign-bidi", "personal_sign", [ + hexlify(toUtf8Bytes(text)), + env.expectedAddress, + ]); + const popup = await waitForApprovalWindow(env.ctx); + await visible(popup, "#view-approve-sign"); + + // The left edge of each character on screen, in byte order. A character + // the browser's fonts draw with no width shares its neighbour's edge. + const lefts = await popup.evaluate(() => { + const message = document.getElementById("approve-sign-message"); + const textNode = message.firstChild; + const range = document.createRange(); + const out = []; + for (let i = 0; i < textNode.length; i++) { + range.setStart(textNode, i); + range.setEnd(textNode, i + 1); + out.push(range.getBoundingClientRect().left); + } + return out; + }); + await clickAndClose(popup, "#btn-reject-sign"); + await assertUserRejection( + env.dapp, + "sign-bidi", + "the byte-order personal_sign rejection", + ); + + assert( + lefts.length === text.length && + lefts.every((left, i) => i === 0 || left >= lefts[i - 1]), + "the personal message is not laid out in byte order: " + + JSON.stringify(lefts), + ); +}); + test("eth_signTypedData_v4 signs, and the signature recovers (#183)", async (env) => { await startRequest(env.dapp, "typed", "eth_signTypedData_v4", [ env.expectedAddress, diff --git a/tests/personalSignDisplay.test.js b/tests/personalSignDisplay.test.js new file mode 100644 index 0000000..4bd0843 --- /dev/null +++ b/tests/personalSignDisplay.test.js @@ -0,0 +1,195 @@ +// The signature prompt shows a personal message as the bytes that are signed +// (https://git.eeqj.de/sneak/AutistMask/issues/403): the raw data in hex, the +// text it decodes to with control characters and characters that paint +// nothing marked rather than obeyed, laid out in byte order, and a message +// that is not hex as plain text that cannot be signed. +// +// Driven against a minimal DOM stub in the shape +// tests/approvalOrigin.test.js uses. That the layout keeps right-to-left +// characters in byte order needs a real browser: tests/e2e/run.js checks it. + +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { hexlify, toUtf8Bytes } = require("ethers"); +const { state } = require("../src/shared/state"); +const approval = require("../src/popup/views/approval"); + +const FROM = "0x0000000000000000000000000000000000000a11"; + +// Built from their code points so that this file holds none of them. +const RIGHT_TO_LEFT_OVERRIDE = String.fromCodePoint(0x202e); +const POP_DIRECTIONAL_FORMATTING = String.fromCodePoint(0x202c); +const ZERO_WIDTH_SPACE = String.fromCodePoint(0x200b); +const VARIATION_SELECTOR_1 = String.fromCodePoint(0xfe00); +const VARIATION_SELECTOR_17 = String.fromCodePoint(0xe0100); +const HANGUL_FILLER = String.fromCodePoint(0x3164); + +function makeElement(id) { + const classes = new Set(); + return { + id, + textContent: "", + value: "", + innerHTML: "", + disabled: false, + style: {}, + dataset: {}, + classList: { + add: (...names) => names.forEach((n) => classes.add(n)), + remove: (...names) => names.forEach((n) => classes.delete(n)), + contains: (n) => classes.has(n), + toggle: (n, force) => { + const on = force === undefined ? !classes.has(n) : force; + if (on) classes.add(n); + else classes.delete(n); + return on; + }, + }, + addEventListener: () => {}, + querySelectorAll: () => [], + appendChild: () => {}, + }; +} + +function makeDocument() { + const els = new Map(); + return { + getElementById(id) { + if (id === "debug-banner") return null; + if (!els.has(id)) els.set(id, makeElement(id)); + return els.get(id); + }, + createElement: () => makeElement("created"), + body: { prepend: () => {} }, + }; +} + +function node(id) { + return globalThis.document.getElementById(id); +} + +// Open the signature prompt for a personal_sign of `message`, the way the +// popup does: it asks the background for the approval and show() draws it. +async function openPersonalSign(message) { + globalThis.document = makeDocument(); + globalThis.window = { location: { search: "" } }; + globalThis.chrome.runtime = { + connect: () => ({ postMessage: () => {} }), + sendMessage: (msg, reply) => { + if (!reply) return; + if (msg.type !== "AUTISTMASK_GET_APPROVAL") return reply(null); + reply({ + type: "sign", + origin: "https://dapp.example", + isPhishingDomain: false, + approvedFrom: FROM, + signParams: { method: "personal_sign", message, from: FROM }, + }); + }, + }; + approval.init({}); + await approval.show(1); +} + +// The message box's markup as the text a reader sees: tags dropped. +function shownMessage() { + return node("approve-sign-message").innerHTML.replace(/<[^>]*>/g, ""); +} + +beforeEach(() => { + state.wallets = []; + state.activeAddress = FROM; + state.viewData = {}; + state.viewStack = []; + state.currentView = null; +}); + +test("a right-to-left override is marked, so the text reads in byte order", async () => { + // Obeyed, the override shows "0001" as "1000". + const text = + "Pay " + + RIGHT_TO_LEFT_OVERRIDE + + "0001" + + POP_DIRECTIONAL_FORMATTING + + " ETH"; + await openPersonalSign(hexlify(toUtf8Bytes(text))); + const html = node("approve-sign-message").innerHTML; + expect(html).not.toContain(RIGHT_TO_LEFT_OVERRIDE); + expect(html).not.toContain(POP_DIRECTIONAL_FORMATTING); + expect(shownMessage()).toBe("Pay U+202E0001U+202C ETH"); +}); + +test("a zero-width character is marked", async () => { + await openPersonalSign( + hexlify(toUtf8Bytes("pay" + ZERO_WIDTH_SPACE + "pal.com")), + ); + expect(node("approve-sign-message").innerHTML).not.toContain( + ZERO_WIDTH_SPACE, + ); + expect(shownMessage()).toBe("payU+200Bpal.com"); +}); + +test("variation selectors and a Hangul filler are marked", async () => { + // Each paints nothing, so a page could hide bytes after "Sign in". + await openPersonalSign( + hexlify( + toUtf8Bytes( + "Sign in" + + VARIATION_SELECTOR_1 + + VARIATION_SELECTOR_17 + + HANGUL_FILLER, + ), + ), + ); + expect(shownMessage()).toBe("Sign inU+FE00U+E0100U+3164"); +}); + +test("the message is laid out in byte order", async () => { + await openPersonalSign(hexlify(toUtf8Bytes("Hello"))); + expect( + node("approve-sign-message").classList.contains("am-byte-order"), + ).toBe(true); +}); + +test("a line feed is shown as a line break", async () => { + await openPersonalSign(hexlify(toUtf8Bytes("Sign in\nNonce: 7"))); + expect(node("approve-sign-message").innerHTML).toBe("Sign in
Nonce: 7"); +}); + +test("the raw hex is shown alongside the text", async () => { + await openPersonalSign("0x48656c6c6f"); + expect(shownMessage()).toBe("Hello"); + expect(node("approve-sign-hex").textContent).toBe("0x48656c6c6f"); + expect(node("approve-sign-hex-section").classList.contains("hidden")).toBe( + false, + ); +}); + +test("hex with an uppercase 0X is read as hex, as signing reads it", async () => { + await openPersonalSign("0X48656C6C6F"); + expect(shownMessage()).toBe("Hello"); + expect(node("approve-sign-hex").textContent).toBe("0X48656C6C6F"); + expect(node("btn-approve-sign").disabled).toBe(false); +}); + +test("bytes that are not text are shown only as hex", async () => { + await openPersonalSign("0xff00"); + expect(node("approve-sign-message").textContent).toBe( + "This message is not text.", + ); + expect(node("approve-sign-hex").textContent).toBe("0xff00"); +}); + +test("a message that is not hex is shown as text and cannot be signed", async () => { + await openPersonalSign("Hello world"); + expect(shownMessage()).toBe("Hello world"); + expect(node("approve-sign-error").textContent).toBe( + "This message is plain text, not hex, so it cannot be signed.", + ); + expect(node("btn-approve-sign").disabled).toBe(true); + expect(node("approve-sign-hex-section").classList.contains("hidden")).toBe( + true, + ); +});