diff --git a/README.md b/README.md index 2a52fc9..4c7d93e 100644 --- a/README.md +++ b/README.md @@ -1920,8 +1920,13 @@ 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 + - Message: for `personal_sign` and `eth_sign`, the text the message's bytes + decode to as UTF-8, with each control or format character (zero-width and + bidirectional characters among them) shown as a bordered `U+XXXX` mark + instead of acting on the text, so it reads in the order of the bytes that + are signed; 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 @@ -1935,12 +1940,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` and an even + number of hex digits) → 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 4ef8c30..d1d82a7 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ 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, control and format + characters in the text are shown as `U+XXXX` marks, and a message that is not + hex is shown as plain text with "Sign" disabled, since signing takes the bytes + from the hex and such a message has none to sign. + - 2026-10-04: A nonce the site supplies with `eth_sendTransaction` is ignored ([#404](https://git.eeqj.de/sneak/AutistMask/issues/404)). It was passed on to the transaction, so a site could replace one of the user's pending 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; } } +// The text as HTML, with each control or format character (zero-width and +// bidirectional characters among them) shown as a bordered U+XXXX mark +// instead of acting on the text, so it reads in the order of its bytes. Line +// feeds are shown as line breaks. +function markControlCharacters(text) { + return escapeHtml(text).replace(/[\p{Cc}\p{Cf}]/gu, (c) => { + if (c === "\n") return "
"; + const code = c.codePointAt(0).toString(16).toUpperCase(); + return `U+${code.padStart(4, "0")}`; + }); +} + // 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 +666,29 @@ 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. 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"); if (isTyped) { $("approve-sign-message").innerHTML = formatTypedDataHtml(sp.typedData); - } else { + refusal = typedDataRefusal(sp); + } else if (isHexString(sp.message, true)) { const decoded = decodeHexMessage(sp.message); if (decoded !== null) { - $("approve-sign-message").textContent = decoded; + $("approve-sign-message").innerHTML = + markControlCharacters(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 = markControlCharacters(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 +710,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/tests/personalSignDisplay.test.js b/tests/personalSignDisplay.test.js new file mode 100644 index 0000000..7f7c9cb --- /dev/null +++ b/tests/personalSignDisplay.test.js @@ -0,0 +1,162 @@ +// 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, zero-width and bidirectional characters +// marked rather than obeyed, 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. + +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); + +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("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("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, + ); +});