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
This commit is contained in:
@@ -2124,7 +2124,12 @@ view would leave a wallet one click from deletion.
|
|||||||
blocklist)
|
blocklist)
|
||||||
- Site origin (bold, scheme and port included) + "wants you to sign a
|
- Site origin (bold, scheme and port included) + "wants you to sign a
|
||||||
message"
|
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)"
|
- Type: "Personal message" or "Typed data (EIP-712)"
|
||||||
- From: color dot + full address + etherscan link
|
- From: color dot + full address + etherscan link
|
||||||
- Message: for `personal_sign` and `eth_sign`, the text the message's bytes
|
- Message: for `personal_sign` and `eth_sign`, the text the message's bytes
|
||||||
|
|||||||
@@ -984,9 +984,8 @@ async function handleRpc(method, params, origin) {
|
|||||||
|
|
||||||
if (method === "eth_sign") {
|
if (method === "eth_sign") {
|
||||||
signParams.dangerWarning =
|
signParams.dangerWarning =
|
||||||
"\u26a0\ufe0f DANGER: This site is requesting to sign a raw hash. " +
|
"This site used the older eth_sign method. " +
|
||||||
"This can be used to sign transactions that drain your funds. " +
|
"The data will be signed as a personal message.";
|
||||||
"Only proceed if you fully understand what you are signing.";
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (findPendingApproval(origin, "sign")) {
|
if (findPendingApproval(origin, "sign")) {
|
||||||
|
|||||||
@@ -721,7 +721,7 @@ function showSignApproval(details) {
|
|||||||
"This message is plain text, not hex, so it cannot be signed.";
|
"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");
|
const warningEl = $("approve-sign-danger-warning");
|
||||||
if (warningEl) {
|
if (warningEl) {
|
||||||
if (sp.dangerWarning) {
|
if (sp.dangerWarning) {
|
||||||
|
|||||||
@@ -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.
|
// The interlock must not cost the retry the approval exists to allow.
|
||||||
describe("the interlock releases a failed attempt", () => {
|
describe("the interlock releases a failed attempt", () => {
|
||||||
test("a retryable failure before the broadcast leaves the approval usable", async () => {
|
test("a retryable failure before the broadcast leaves the approval usable", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user