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, line and paragraph separator, and 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
2026-10-04 19:44:03 +00:00
parent 3b713809c8
commit 0671f34ecc
8 changed files with 401 additions and 16 deletions
@@ -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,
@@ -0,0 +1,229 @@
+// 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, markup shown as
+// text, 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<br>Nonce: 7");
+});
+
+// The message box is written as HTML, so a site's markup has to arrive there
+// escaped, as the text it is.
+const MARKUP = "<b>x</b><img src=x onerror=alert(1)>";
+
+test.each([
+    ["a hex message", hexlify(toUtf8Bytes(MARKUP))],
+    ["a message that is not hex", MARKUP],
+])("markup in %s is shown as text, not as markup", async (_, message) => {
+    await openPersonalSign(message);
+    expect(node("approve-sign-message").innerHTML).toBe(
+        "&lt;b&gt;x&lt;/b&gt;&lt;img src=x onerror=alert(1)&gt;",
+    );
+});
+
+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,
+    );
+});