diff --git a/README.md b/README.md index 9c4364e..aa1891c 100644 --- a/README.md +++ b/README.md @@ -1927,10 +1927,19 @@ 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, + each line or paragraph separator (U+2028, U+2029; left in the text, a + paragraph separator would end that layout for the text after it), 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 +1951,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..01ad8ee 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, line and paragraph separators 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), each control character and each line or paragraph separator +// (U+2028, U+2029) shown as a mark. A line feed is shown as a line break. +// Left in the text, a paragraph separator would end the byte-order layout +// for everything after it. 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}\p{Zl}\p{Zp}]/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 +686,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 +734,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..4990bc6 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -3232,6 +3232,64 @@ 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. A +// paragraph separator (U+2029) before it, left in the text, would end the +// byte-order layout and bring that back +// (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 rightToLeft = String.fromCodePoint(0x05c3); + const text = + "Sign in" + + String.fromCodePoint(0x2029) + + "Pay 5" + + rightToLeft + + "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 text on screen, marks included, and the left edge of each of its + // characters, in byte order. A character the browser's fonts draw with + // no width shares its neighbour's edge. + const shown = await popup.evaluate(() => { + const message = document.getElementById("approve-sign-message"); + const walker = document.createTreeWalker(message, NodeFilter.SHOW_TEXT); + const range = document.createRange(); + let text = ""; + const lefts = []; + for (let node = walker.nextNode(); node; node = walker.nextNode()) { + for (let i = 0; i < node.length; i++) { + range.setStart(node, i); + range.setEnd(node, i + 1); + lefts.push(range.getBoundingClientRect().left); + } + text += node.data; + } + return { text, lefts }; + }); + await clickAndClose(popup, "#btn-reject-sign"); + await assertUserRejection( + env.dapp, + "sign-bidi", + "the byte-order personal_sign rejection", + ); + + assert( + shown.text === "Sign inU+2029Pay 5" + rightToLeft + "00 ETH", + "the paragraph separator is not shown as a mark: " + + JSON.stringify(shown.text), + ); + assert( + shown.lefts.every((left, i) => i === 0 || left >= shown.lefts[i - 1]), + "the personal message is not laid out in byte order: " + + JSON.stringify(shown.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..0ca7b0f --- /dev/null +++ b/tests/personalSignDisplay.test.js @@ -0,0 +1,215 @@ +// 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, line and paragraph separators +// 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); +const LINE_SEPARATOR = String.fromCodePoint(0x2028); +const PARAGRAPH_SEPARATOR = String.fromCodePoint(0x2029); + +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 control character other than a line feed is marked", async () => { + await openPersonalSign(hexlify(toUtf8Bytes("a\u0000b\tc"))); + expect(shownMessage()).toBe("aU+0000bU+0009c"); +}); + +test("line and paragraph separators are marked", async () => { + // Left in the text, a paragraph separator would end the byte-order + // layout for everything after it. + await openPersonalSign( + hexlify(toUtf8Bytes("a" + LINE_SEPARATOR + "b" + PARAGRAPH_SEPARATOR)), + ); + const html = node("approve-sign-message").innerHTML; + expect(html).not.toContain(LINE_SEPARATOR); + expect(html).not.toContain(PARAGRAPH_SEPARATOR); + expect(shownMessage()).toBe("aU+2028bU+2029"); +}); + +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, + ); +});