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 () => {