From 00d1fecc366a7a119c1f41c0077d5ecf450cdf0d Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 15:41:22 +0200 Subject: [PATCH] fix: eth_sign's warning and the README say what it signs (closes #409) eth_sign is signed as an EIP-191 personal message over its data, which is what the Ethereum JSON-RPC specification defines it to return. Its warning on the signature prompt said the site was asking to sign a raw hash that could drain funds, and the README said the same; neither was true. The warning now says the site used the older eth_sign method and that the data will be signed as a personal message, and the README's SignApproval section says so too. Tests pin that the background reads eth_sign address first and data second, and that the sign screen answers it with the personal message signature over the data. Model: opus-5-5 --- README.md | 7 ++- src/background/index.js | 5 +- src/popup/views/approval.js | 2 +- tests/backgroundApproval.test.js | 49 +++++++++++++++ tests/ethSign.test.js | 102 +++++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 tests/ethSign.test.js diff --git a/README.md b/README.md index 49c38b0..b8fb070 100644 --- a/README.md +++ b/README.md @@ -2136,7 +2136,12 @@ view would leave a wallet one click from deletion. blocklist) - Site origin (bold, scheme and port included) + "wants you to sign a message" - - Danger warning box (shown for `eth_sign`, which signs a raw hash) + - Warning box, shown for `eth_sign`: "This site used the older eth_sign + method. The data will be signed as a personal message." An `eth_sign` + request names the address first and the data second. Its data is signed + exactly as a `personal_sign` message is, as an EIP-191 personal message, + which is what the Ethereum JSON-RPC specification defines `eth_sign` to + return. - Type: "Personal message" or "Typed data (EIP-712)" - From: color dot + full address + etherscan link - Message: for `personal_sign` and `eth_sign`, the text the message's bytes diff --git a/src/background/index.js b/src/background/index.js index 00fa439..2b55689 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -988,9 +988,8 @@ async function handleRpc(method, params, origin) { if (method === "eth_sign") { signParams.dangerWarning = - "\u26a0\ufe0f DANGER: This site is requesting to sign a raw hash. " + - "This can be used to sign transactions that drain your funds. " + - "Only proceed if you fully understand what you are signing."; + "This site used the older eth_sign method. " + + "The data will be signed as a personal message."; } if (findPendingApproval(origin, "sign")) { diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 823e465..4a65d90 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -725,7 +725,7 @@ function showSignApproval(details) { "This message is plain text, not hex, so it cannot be signed."; } - // Display danger warning for eth_sign (raw hash signing) + // The warning the background attaches to an eth_sign request. const warningEl = $("approve-sign-danger-warning"); if (warningEl) { if (sp.dangerWarning) { diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index f0ad395..07218fd 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -1606,6 +1606,55 @@ describe("what the approval is verified against", () => { }); }); +// eth_sign names the address first and the data second, the reverse of +// personal_sign, and is answered as the JSON-RPC specification defines it: +// with an EIP-191 personal message signature over the data, the same +// signature personal_sign returns +// (https://git.eeqj.de/sneak/AutistMask/issues/409). The signature below is +// made by the test; that the sign screen makes the same one is tested in +// tests/ethSign.test.js. +describe("eth_sign", () => { + test("is read address first, and returns a personal message signature over the data", async () => { + const bg = loadBackground(); + const page = bg.send( + { + type: "AUTISTMASK_RPC", + method: "eth_sign", + params: [signer.address, MESSAGE], + }, + { origin: ORIGIN }, + ); + await settle(); + const id = approvalIdIn(bg.created.map((c) => c.url)); + + const details = bg.send( + { type: "AUTISTMASK_GET_APPROVAL", id }, + { url: bg.fromPopup.url }, + ); + const { signParams } = details.sendResponse.mock.calls[0][0]; + expect(signParams).toMatchObject({ + method: "eth_sign", + message: MESSAGE, + from: signer.address, + }); + expect(signParams.dangerWarning).toBe( + "This site used the older eth_sign method. " + + "The data will be signed as a personal message.", + ); + + const signature = await signer.signMessage( + Buffer.from(MESSAGE.slice(2), "hex"), + ); + bg.send( + { type: "AUTISTMASK_SIGN_RESPONSE", id, approved: true, signature }, + { url: bg.fromPopup.url }, + ); + await settle(); + + expect(page.sendResponse).toHaveBeenCalledWith({ result: signature }); + }); +}); + // The interlock must not cost the retry the approval exists to allow. describe("the interlock releases a failed attempt", () => { test("a retryable failure before the broadcast leaves the approval usable", async () => { diff --git a/tests/ethSign.test.js b/tests/ethSign.test.js new file mode 100644 index 0000000..5297a5f --- /dev/null +++ b/tests/ethSign.test.js @@ -0,0 +1,102 @@ +// The sign screen signs an eth_sign request the way it signs personal_sign: as +// an EIP-191 personal message over the bytes of the data +// (https://git.eeqj.de/sneak/AutistMask/issues/409). Booted through the real +// popup entry point as the approval window. The background's half, which reads +// the address first and the data second and checks the signature, is tested in +// tests/backgroundApproval.test.js. + +// Mocked so that no password has to be hashed: the stored secret decrypts to +// the test key below. +jest.mock("../src/shared/vault", () => ({ + decryptWithPassword: jest.fn(), + encryptWithPassword: jest.fn(), +})); + +const { Wallet, getBytes } = require("ethers"); +const { + bootPopup, + cleanupPopup, + unversionedValidProfile, +} = require("./support/popupBoot"); + +// A published test key, never used for real funds. +const SIGNER_KEY = + "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; +const signer = new Wallet(SIGNER_KEY); +// "Hello AutistMask", as the hex a page sends. +const MESSAGE = "0x48656c6c6f204175746973744d61736b"; +const ORIGIN = "https://dapp.example"; + +afterEach(cleanupPopup); + +test("the sign screen answers eth_sign with a personal message signature over the data", async () => { + const sent = []; + const page = await bootPopup( + unversionedValidProfile({ + wallets: [ + { + type: "key", + name: "Wallet 1", + encryptedSecret: "encrypted-key", + addresses: [ + { + address: signer.address, + balance: "0.0000", + tokenBalances: [], + }, + ], + }, + ], + activeAddress: signer.address, + allowedSites: { [signer.address]: [ORIGIN] }, + }), + { + search: "?approval=approval-1", + runtime: { + connect: () => ({ postMessage: () => {} }), + // Answered as the background answers: with the eth_sign + // approval this window was opened for, and by handing back + // the signature the popup sends. + sendMessage: (msg, reply) => { + sent.push(msg); + if (msg.type === "AUTISTMASK_GET_APPROVAL") { + reply({ + type: "sign", + origin: ORIGIN, + isPhishingDomain: false, + approvedFrom: signer.address, + signParams: { + method: "eth_sign", + message: MESSAGE, + from: signer.address, + }, + }); + } else if (msg.type === "AUTISTMASK_SIGN_RESPONSE") { + reply({ signature: msg.signature }); + } else { + reply(undefined); + } + }, + }, + }, + ); + globalThis.window.close = jest.fn(); + const { decryptWithPassword } = require("../src/shared/vault"); + decryptWithPassword.mockResolvedValue(SIGNER_KEY); + + expect(page.visibleViews()).toEqual(["approve-sign"]); + page.node("approve-sign-password").value = "any password"; + await page.click("btn-approve-sign"); + + expect( + sent.filter((msg) => msg.type === "AUTISTMASK_SIGN_RESPONSE"), + ).toEqual([ + { + type: "AUTISTMASK_SIGN_RESPONSE", + id: "approval-1", + approved: true, + signature: await signer.signMessage(getBytes(MESSAGE)), + }, + ]); + expect(globalThis.window.close).toHaveBeenCalled(); +});