From f0a2d150a2472883010f04434f90fe33939d18d0 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 02:12:23 +0000 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. A test pins that eth_sign is read address first and data second, and that the page receives 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 | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a4505e2..7b528ed 100644 --- a/README.md +++ b/README.md @@ -2124,7 +2124,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 d5a6fc4..8d26af4 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -984,9 +984,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 166a4a5..58729bf 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -721,7 +721,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 40d3e6c..d364691 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -1551,6 +1551,53 @@ 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). +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 () => {