diff --git a/TODO.md b/TODO.md index 5677a1b..c4fb605 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,10 @@ get review, merge to main. # Completed Steps +- 2026-08-09: dApp approval signing moved into the popup — the password no + longer crosses the extension messaging boundary; the background broadcasts and + resolves approvals only, and verifies the signed artifact against the approval + it holds (#157). - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-03-01: About well in settings with build info and easter egg (in flight diff --git a/src/background/index.js b/src/background/index.js index 9a4879f..159ed55 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -5,7 +5,6 @@ const { DEFAULT_RPC_URL } = require("../shared/constants"); const { SUPPORTED_CHAIN_IDS, networkByChainId } = require("../shared/networks"); const { onChainSwitch } = require("../shared/chainSwitch"); -const { getBytes } = require("ethers"); const { state, loadState, @@ -14,8 +13,7 @@ const { } = require("../shared/state"); const { refreshBalances, getProvider } = require("../shared/balances"); const { debugFetch } = require("../shared/log"); -const { decryptWithPassword } = require("../shared/vault"); -const { getSignerForAddress } = require("../shared/wallet"); +const { verifySignedTx, verifySignature } = require("../shared/approvalVerify"); const { isPhishingDomain, updatePhishingList, @@ -725,39 +723,28 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { return true; } + // The popup signs; it reports back here when it could not. Fail the + // request the same way this handler used to when it did the signing. + if (msg.error) { + approval.resolve({ error: { message: msg.error } }); + sendResponse({ error: msg.error }); + return false; + } + (async () => { try { await loadState(); const activeAddress = await getActiveAddress(); - let wallet, addrIndex; - for (const w of state.wallets) { - for (let i = 0; i < w.addresses.length; i++) { - if (w.addresses[i].address === activeAddress) { - wallet = w; - addrIndex = i; - break; - } - } - if (wallet) break; - } - if (!wallet) throw new Error("Wallet not found"); - // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage - let decrypted = await decryptWithPassword( - wallet.encryptedSecret, - msg.password, + // The popup holds the secret, but the background stays the + // authority on what is broadcast: the raw transaction must be + // the approved one, signed by the approved address. + verifySignedTx( + msg.rawSignedTx, + approval.txParams, + activeAddress, ); - const signer = getSignerForAddress( - wallet, - addrIndex, - decrypted, - ); - // Best-effort: clear decrypted secret after use. - // Note: JS strings are immutable; this nulls the reference but - // the original string may persist in memory until GC. - decrypted = null; const provider = getProvider(state.rpcUrl); - const connected = signer.connect(provider); - const tx = await connected.sendTransaction(approval.txParams); + const tx = await provider.broadcastTransaction(msg.rawSignedTx); approval.resolve({ txHash: tx.hash }); sendResponse({ txHash: tx.hash }); } catch (e) { @@ -784,55 +771,23 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { return true; } + // The popup signs; it reports back here when it could not. Fail the + // request the same way this handler used to when it did the signing. + if (msg.error) { + approval.resolve({ error: { message: msg.error } }); + sendResponse({ error: msg.error }); + return false; + } + (async () => { try { - await loadState(); const activeAddress = await getActiveAddress(); - let wallet, addrIndex; - for (const w of state.wallets) { - for (let i = 0; i < w.addresses.length; i++) { - if (w.addresses[i].address === activeAddress) { - wallet = w; - addrIndex = i; - break; - } - } - if (wallet) break; - } - if (!wallet) throw new Error("Wallet not found"); - // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage - let decrypted = await decryptWithPassword( - wallet.encryptedSecret, - msg.password, - ); - const signer = getSignerForAddress( - wallet, - addrIndex, - decrypted, - ); - // Best-effort: clear decrypted secret after use. - // Note: JS strings are immutable; this nulls the reference but - // the original string may persist in memory until GC. - decrypted = null; - - const sp = approval.signParams; - let signature; - - if (sp.method === "personal_sign" || sp.method === "eth_sign") { - signature = await signer.signMessage(getBytes(sp.message)); - } else { - // eth_signTypedData_v4 / eth_signTypedData - const typedData = JSON.parse(sp.typedData); - const { domain, types, message } = typedData; - // ethers handles EIP712Domain internally - delete types.EIP712Domain; - signature = await signer.signTypedData( - domain, - types, - message, - ); - } - + // The popup holds the secret, but the background stays the + // authority on what is handed back to the page: the signature + // must cover the approved payload and recover to the approved + // address. + const signature = msg.signature; + verifySignature(approval.signParams, signature, activeAddress); approval.resolve({ signature }); sendResponse({ signature }); } catch (e) { diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 425a6f4..c9cc740 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -9,10 +9,19 @@ const { attachCopyHandlers, } = require("./helpers"); const { state, saveState, currentNetwork } = require("../../shared/state"); -const { formatEther, formatUnits, Interface, toUtf8String } = require("ethers"); +const { + formatEther, + formatUnits, + getBytes, + Interface, + toUtf8String, +} = require("ethers"); const { getPrice, formatUsd } = require("../../shared/prices"); const { ERC20_ABI } = require("../../shared/constants"); const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); +const { decryptWithPassword } = require("../../shared/vault"); +const { getSignerForAddress } = require("../../shared/wallet"); +const { getProvider } = require("../../shared/balances"); const txStatus = require("./txStatus"); const uniswap = require("../../shared/uniswap"); const runtime = @@ -153,6 +162,8 @@ function showTxApproval(details) { details.isPhishingDomain, ); + pendingTxParams = details.txParams; + const toAddr = details.txParams.to; const token = toAddr ? TOKEN_BY_ADDRESS.get(toAddr.toLowerCase()) : null; const ethValue = formatEther(details.txParams.value || "0"); @@ -326,6 +337,7 @@ function showSignApproval(details) { ); const sp = details.signParams; + pendingSignParams = sp; $("approve-sign-hostname").textContent = details.hostname; $("approve-sign-from").innerHTML = approvalAddressHtml(sp.from); @@ -401,6 +413,36 @@ function show(id) { let approvalId = null; let pendingTxDetails = null; +// The exact parameters shown to the user, kept so the popup signs what it +// displayed rather than re-fetching anything at approval time. Both are +// repopulated by show() when the popup is closed and reopened. +let pendingTxParams = null; +let pendingSignParams = null; + +// Approve buttons stay disabled and muted while the popup derives the key and +// signs, which is slow enough (Argon2id) that a double click is likely. +function setTxButtonBusy(busy) { + $("btn-approve-tx").disabled = busy; + $("btn-approve-tx").classList.toggle("text-muted", busy); +} + +function setSignButtonBusy(busy) { + $("btn-approve-sign").disabled = busy; + $("btn-approve-sign").classList.toggle("text-muted", busy); +} + +// Locate the wallet and the address index owning the currently active +// address. Returns null when no wallet holds it. +function findActiveWallet() { + for (const wallet of state.wallets) { + for (let i = 0; i < wallet.addresses.length; i++) { + if (wallet.addresses[i].address === state.activeAddress) { + return { wallet, addrIndex: i }; + } + } + } + return null; +} function init(ctx) { $("approve-remember").addEventListener("change", async () => { @@ -430,34 +472,86 @@ function init(ctx) { window.close(); }); - $("btn-approve-tx").addEventListener("click", () => { - const password = $("approve-tx-password").value; + $("btn-approve-tx").addEventListener("click", async () => { + let password = $("approve-tx-password").value; if (!password) { showError("approve-tx-error", "Please enter your password."); return; } hideError("approve-tx-error"); - $("btn-approve-tx").disabled = true; - $("btn-approve-tx").classList.add("text-muted"); + setTxButtonBusy(true); - runtime.sendMessage( - { - type: "AUTISTMASK_TX_RESPONSE", - id: approvalId, - approved: true, - // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage - password: password, - }, - (response) => { - if (response && response.txHash) { - txStatus.showWait(pendingTxDetails, response.txHash); - } else { - const msg = - (response && response.error) || "Transaction failed."; - txStatus.showError(pendingTxDetails, null, msg); - } - }, - ); + const active = findActiveWallet(); + if (!active) { + password = null; + showError( + "approve-tx-error", + "No wallet was found for the active address.", + ); + setTxButtonBusy(false); + return; + } + + // Decrypt here, in the popup. The password must never cross the + // extension messaging boundary; only the signed transaction does. + let decryptedSecret; + try { + decryptedSecret = await decryptWithPassword( + active.wallet.encryptedSecret, + password, + ); + } catch { + showError( + "approve-tx-error", + "That password is incorrect. Please try again.", + ); + setTxButtonBusy(false); + return; + } finally { + // Best-effort: drop the password as soon as the key derivation + // is done. Note that JS strings are immutable; this clears the + // reference but the original string may persist until GC. + password = null; + } + + const payload = { + type: "AUTISTMASK_TX_RESPONSE", + id: approvalId, + approved: true, + }; + try { + const signer = getSignerForAddress( + active.wallet, + active.addrIndex, + decryptedSecret, + ); + const provider = getProvider(state.rpcUrl); + const connected = signer.connect(provider); + // This is the sequence ethers' own sendTransaction() runs + // internally, so nonce, gas, fee and chain id population are + // identical to when the background did the signing. + const populated = + await connected.populateTransaction(pendingTxParams); + delete populated.from; + payload.rawSignedTx = await connected.signTransaction(populated); + } catch (e) { + payload.error = + e.shortMessage || e.message || "Transaction signing failed."; + } finally { + // Best-effort: clear the decrypted secret after use, with the + // same immutability caveat as the password above. + decryptedSecret = null; + } + + runtime.sendMessage(payload, (response) => { + if (response && response.txHash) { + txStatus.showWait(pendingTxDetails, response.txHash); + } else { + const msg = + (response && response.error) || "Transaction failed."; + txStatus.showError(pendingTxDetails, null, msg); + } + }); }); $("btn-reject-tx").addEventListener("click", () => { @@ -469,36 +563,93 @@ function init(ctx) { window.close(); }); - $("btn-approve-sign").addEventListener("click", () => { - const password = $("approve-sign-password").value; + $("btn-approve-sign").addEventListener("click", async () => { + let password = $("approve-sign-password").value; if (!password) { showError("approve-sign-error", "Please enter your password."); return; } hideError("approve-sign-error"); - $("btn-approve-sign").disabled = true; - $("btn-approve-sign").classList.add("text-muted"); + setSignButtonBusy(true); - runtime.sendMessage( - { - type: "AUTISTMASK_SIGN_RESPONSE", - id: approvalId, - approved: true, - // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage - password: password, - }, - (response) => { - if (response && response.signature) { - window.close(); - } else { - const msg = - (response && response.error) || "Signing failed."; - showError("approve-sign-error", msg); - $("btn-approve-sign").disabled = false; - $("btn-approve-sign").classList.remove("text-muted"); - } - }, - ); + const active = findActiveWallet(); + if (!active) { + password = null; + showError( + "approve-sign-error", + "No wallet was found for the active address.", + ); + setSignButtonBusy(false); + return; + } + + // Decrypt here, in the popup. The password must never cross the + // extension messaging boundary; only the signature does. + let decryptedSecret; + try { + decryptedSecret = await decryptWithPassword( + active.wallet.encryptedSecret, + password, + ); + } catch { + showError( + "approve-sign-error", + "That password is incorrect. Please try again.", + ); + setSignButtonBusy(false); + return; + } finally { + // Best-effort: drop the password as soon as the key derivation + // is done. Note that JS strings are immutable; this clears the + // reference but the original string may persist until GC. + password = null; + } + + const payload = { + type: "AUTISTMASK_SIGN_RESPONSE", + id: approvalId, + approved: true, + }; + try { + const signer = getSignerForAddress( + active.wallet, + active.addrIndex, + decryptedSecret, + ); + const sp = pendingSignParams; + if (sp.method === "personal_sign" || sp.method === "eth_sign") { + payload.signature = await signer.signMessage( + getBytes(sp.message), + ); + } else { + // eth_signTypedData_v4 / eth_signTypedData + const typedData = JSON.parse(sp.typedData); + const { domain, types, message } = typedData; + // ethers handles EIP712Domain internally + delete types.EIP712Domain; + payload.signature = await signer.signTypedData( + domain, + types, + message, + ); + } + } catch (e) { + payload.error = e.shortMessage || e.message || "Signing failed."; + } finally { + // Best-effort: clear the decrypted secret after use, with the + // same immutability caveat as the password above. + decryptedSecret = null; + } + + runtime.sendMessage(payload, (response) => { + if (response && response.signature) { + window.close(); + } else { + const msg = (response && response.error) || "Signing failed."; + showError("approve-sign-error", msg); + setSignButtonBusy(false); + } + }); }); $("btn-reject-sign").addEventListener("click", () => { diff --git a/src/shared/approvalVerify.js b/src/shared/approvalVerify.js new file mode 100644 index 0000000..4004425 --- /dev/null +++ b/src/shared/approvalVerify.js @@ -0,0 +1,124 @@ +// Verification of the signed artifacts produced by the approval popup. +// +// Signing happens in the popup, where the password is entered; the background +// only broadcasts the raw transaction and resolves the pending approval back +// to the requesting page. So that moving the signing out of the background +// does not turn the background into a blind relay, the background re-derives +// the signer from the artifact and checks it against the approval it is +// holding before acting on it. All recovery is delegated to ethers. +// +// Every failure message is a full sentence, because these strings are shown to +// the user and returned to the dApp. + +const { + Transaction, + getAddress, + getBytes, + verifyMessage, + verifyTypedData, +} = require("ethers"); + +// Case-insensitive address comparison that tolerates absent values on either +// side. Two absent addresses compare equal (contract creation has no `to`). +function sameAddress(a, b) { + const aMissing = a === null || a === undefined || a === ""; + const bMissing = b === null || b === undefined || b === ""; + if (aMissing || bMissing) return aMissing && bMissing; + try { + return getAddress(a) === getAddress(b); + } catch { + return String(a).toLowerCase() === String(b).toLowerCase(); + } +} + +// Normalize a transaction value (hex string, decimal string, number or +// bigint) to a bigint. An absent value is zero, matching ethers. +function normalizeValue(v) { + if (v === null || v === undefined || v === "") return 0n; + return BigInt(v); +} + +// Normalize call data to a lowercase hex string. Absent data is "0x". +function normalizeData(v) { + if (v === null || v === undefined || v === "" || v === "0x") return "0x"; + return String(v).toLowerCase(); +} + +// Assert that a raw signed transaction is the transaction the user approved, +// signed by the address the approval was raised for. Returns the parsed +// ethers Transaction on success, throws otherwise. +function verifySignedTx(rawSignedTx, txParams, expectedFrom) { + if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) { + throw new Error("The signed transaction is missing or malformed."); + } + + let parsed; + try { + parsed = Transaction.from(rawSignedTx); + } catch { + throw new Error("The signed transaction could not be decoded."); + } + + if (!parsed.from) { + throw new Error("The signed transaction carries no valid signature."); + } + if (!sameAddress(parsed.from, expectedFrom)) { + throw new Error( + "The signed transaction was signed by a different address than the one that was approved.", + ); + } + if (!sameAddress(parsed.to, txParams.to)) { + throw new Error( + "The signed transaction does not go to the approved recipient.", + ); + } + if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) { + throw new Error( + "The signed transaction does not carry the approved value.", + ); + } + if (normalizeData(parsed.data) !== normalizeData(txParams.data)) { + throw new Error( + "The signed transaction does not carry the approved call data.", + ); + } + + return parsed; +} + +// Assert that a signature over the approved message or typed data was +// produced by the address the approval was raised for. Returns the recovered +// address on success, throws otherwise. +function verifySignature(signParams, signature, expectedFrom) { + if (typeof signature !== "string" || !signature.startsWith("0x")) { + throw new Error("The signature is missing or malformed."); + } + + let recovered; + try { + if ( + signParams.method === "personal_sign" || + signParams.method === "eth_sign" + ) { + recovered = verifyMessage(getBytes(signParams.message), signature); + } else { + const typedData = JSON.parse(signParams.typedData); + const { domain, types, message } = typedData; + // ethers derives EIP712Domain itself and rejects it as an input. + delete types.EIP712Domain; + recovered = verifyTypedData(domain, types, message, signature); + } + } catch { + throw new Error("The signature could not be verified."); + } + + if (!sameAddress(recovered, expectedFrom)) { + throw new Error( + "The signature was produced by a different address than the one that was approved.", + ); + } + + return recovered; +} + +module.exports = { verifySignedTx, verifySignature, sameAddress }; diff --git a/tests/approvalVerify.test.js b/tests/approvalVerify.test.js new file mode 100644 index 0000000..1146416 --- /dev/null +++ b/tests/approvalVerify.test.js @@ -0,0 +1,355 @@ +const { Network, Transaction, Wallet } = require("ethers"); +const { + verifySignedTx, + verifySignature, + sameAddress, +} = require("../src/shared/approvalVerify"); +const { getSignerForAddress } = require("../src/shared/wallet"); + +// Fixed test keys — never used for anything but these tests. +const SIGNER_KEY = + "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; +const OTHER_KEY = + "0x5de4111afa1a4b94908f83103eb1f1706367c2e68ca870fc3fb9a804cdab365a"; + +const signer = new Wallet(SIGNER_KEY); +const other = new Wallet(OTHER_KEY); + +const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; +const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; + +// Approved parameters as a dApp would supply them over eth_sendTransaction. +const TX_PARAMS = { + from: signer.address, + to: RECIPIENT, + value: "0x2386f26fc10000", + data: "0xdeadbeef", + gas: "0x5208", +}; + +// Build a signable transaction from approved params. The popup does the same +// thing through populateTransaction(); here the fields are fixed so the test +// needs no provider. +function txFor(params) { + return { + chainId: 1, + nonce: 7, + gasLimit: 100000n, + maxFeePerGas: 2000000000n, + maxPriorityFeePerGas: 1000000000n, + type: 2, + to: params.to, + value: params.value === undefined ? 0n : BigInt(params.value), + data: params.data || "0x", + }; +} + +async function signedFor(params, withWallet) { + return (withWallet || signer).signTransaction(txFor(params)); +} + +describe("sameAddress", () => { + test("compares checksummed and lowercase forms as equal", () => { + expect(sameAddress(RECIPIENT, RECIPIENT.toLowerCase())).toBe(true); + }); + + test("treats two absent addresses as equal (contract creation)", () => { + expect(sameAddress(null, undefined)).toBe(true); + expect(sameAddress("", null)).toBe(true); + }); + + test("treats one absent address as unequal", () => { + expect(sameAddress(RECIPIENT, null)).toBe(false); + expect(sameAddress(null, RECIPIENT)).toBe(false); + }); + + test("does not throw on values that are not addresses", () => { + expect(sameAddress("not-an-address", RECIPIENT)).toBe(false); + }); +}); + +describe("verifySignedTx", () => { + test("accepts the approved transaction signed by the approved address", async () => { + const raw = await signedFor(TX_PARAMS); + const parsed = verifySignedTx(raw, TX_PARAMS, signer.address); + expect(parsed.from).toBe(signer.address); + expect(parsed.hash).toBe(Transaction.from(raw).hash); + }); + + test("accepts a contract creation with no recipient", async () => { + const params = { to: undefined, value: "0x0", data: "0x600160005500" }; + const raw = await signedFor(params); + expect(() => verifySignedTx(raw, params, signer.address)).not.toThrow(); + }); + + test("accepts an absent value as zero", async () => { + const approved = { to: RECIPIENT, data: "0x" }; + const raw = await signedFor(approved); + expect(() => + verifySignedTx(raw, approved, signer.address), + ).not.toThrow(); + }); + + test("accepts call data whose case differs from the approval", async () => { + const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" }; + const raw = await signedFor(approved); + expect(() => + verifySignedTx(raw, approved, signer.address), + ).not.toThrow(); + }); + + test("rejects a swapped recipient", async () => { + const raw = await signedFor({ + ...TX_PARAMS, + to: OTHER_RECIPIENT, + }); + expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( + /approved recipient/, + ); + }); + + test("rejects an inflated value", async () => { + const raw = await signedFor({ + ...TX_PARAMS, + value: "0x4563918244f40000", + }); + expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( + /approved value/, + ); + }); + + test("rejects substituted call data", async () => { + const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" }); + expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( + /approved call data/, + ); + }); + + test("rejects a transaction signed by a different address", async () => { + const raw = await signedFor(TX_PARAMS, other); + expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( + /different address/, + ); + }); + + test("rejects an unsigned transaction", () => { + const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized; + expect(() => + verifySignedTx(unsigned, TX_PARAMS, signer.address), + ).toThrow(/no valid signature/); + }); + + test("rejects a missing or malformed payload", () => { + expect(() => + verifySignedTx(undefined, TX_PARAMS, signer.address), + ).toThrow(/missing or malformed/); + expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow( + /missing or malformed/, + ); + expect(() => + verifySignedTx("0xc0ffee", TX_PARAMS, signer.address), + ).toThrow(/could not be decoded/); + }); + + test("every rejection message is a full sentence", async () => { + const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT }); + try { + verifySignedTx(raw, TX_PARAMS, signer.address); + throw new Error("expected a rejection"); + } catch (e) { + expect(e.message).toMatch(/^[A-Z].*\.$/); + } + }); +}); + +const TYPED_DATA = JSON.stringify({ + domain: { + name: "AutistMask Test", + version: "1", + chainId: 1, + verifyingContract: OTHER_RECIPIENT, + }, + primaryType: "Mail", + types: { + EIP712Domain: [ + { name: "name", type: "string" }, + { name: "version", type: "string" }, + { name: "chainId", type: "uint256" }, + { name: "verifyingContract", type: "address" }, + ], + Mail: [ + { name: "from", type: "address" }, + { name: "to", type: "address" }, + { name: "contents", type: "string" }, + ], + }, + message: { + from: signer.address, + to: RECIPIENT, + contents: "hello", + }, +}); + +describe("verifySignature", () => { + // "Hello AutistMask" as the hex string a dApp passes to personal_sign. + const MESSAGE = "0x48656c6c6f204175746973744d61736b"; + const personalParams = { + method: "personal_sign", + message: MESSAGE, + from: signer.address, + }; + const typedParams = { + method: "eth_signTypedData_v4", + typedData: TYPED_DATA, + from: signer.address, + }; + + async function signPersonal(withWallet) { + return (withWallet || signer).signMessage( + Buffer.from(MESSAGE.slice(2), "hex"), + ); + } + + async function signTyped(withWallet) { + const { domain, types, message } = JSON.parse(TYPED_DATA); + delete types.EIP712Domain; + return (withWallet || signer).signTypedData(domain, types, message); + } + + test("accepts a personal_sign signature from the approved address", async () => { + const signature = await signPersonal(); + expect(verifySignature(personalParams, signature, signer.address)).toBe( + signer.address, + ); + }); + + test("accepts an eth_sign signature the same way", async () => { + const signature = await signPersonal(); + const params = { ...personalParams, method: "eth_sign" }; + expect(() => + verifySignature(params, signature, signer.address), + ).not.toThrow(); + }); + + test("accepts a typed data signature from the approved address", async () => { + const signature = await signTyped(); + expect(verifySignature(typedParams, signature, signer.address)).toBe( + signer.address, + ); + }); + + test("does not mutate the approved typed data while verifying", async () => { + const signature = await signTyped(); + const before = typedParams.typedData; + verifySignature(typedParams, signature, signer.address); + expect(typedParams.typedData).toBe(before); + expect( + JSON.parse(typedParams.typedData).types.EIP712Domain, + ).toBeDefined(); + }); + + test("rejects a personal_sign signature from a different address", async () => { + const signature = await signPersonal(other); + expect(() => + verifySignature(personalParams, signature, signer.address), + ).toThrow(/different address/); + }); + + test("rejects a typed data signature from a different address", async () => { + const signature = await signTyped(other); + expect(() => + verifySignature(typedParams, signature, signer.address), + ).toThrow(/different address/); + }); + + test("rejects a signature over a different message", async () => { + const signature = await signer.signMessage( + Buffer.from("00112233", "hex"), + ); + expect(() => + verifySignature(personalParams, signature, signer.address), + ).toThrow(/different address/); + }); + + test("rejects a missing or malformed signature", async () => { + expect(() => + verifySignature(personalParams, undefined, signer.address), + ).toThrow(/missing or malformed/); + expect(() => + verifySignature(personalParams, "0x1234", signer.address), + ).toThrow(/could not be verified/); + }); +}); + +// End-to-end over the messaging boundary, without a browser: run the exact +// sequence the approval popup runs, then hand the artifact to the exact check +// the background runs before it broadcasts or resolves. Only what the popup +// puts on the wire is passed along, so this also pins down that the wire +// payload is sufficient on its own. +describe("popup signing sequence to background verification", () => { + // Stand-in for the JSON-RPC provider. populateTransaction only needs the + // nonce, the gas estimate, the network and the fee data. + const fakeProvider = { + getNetwork: async () => Network.from(1), + getTransactionCount: async () => 7, + estimateGas: async () => 21000n, + getFeeData: async () => ({ + gasPrice: 2000000000n, + maxFeePerGas: 2000000000n, + maxPriorityFeePerGas: 1000000000n, + }), + }; + + // A private-key wallet as it is persisted in state, so the test goes + // through getSignerForAddress() the way the popup does. + const walletData = { type: "privkey" }; + + async function popupSignsTx(txParams) { + const localSigner = getSignerForAddress(walletData, 0, SIGNER_KEY); + const connected = localSigner.connect(fakeProvider); + const populated = await connected.populateTransaction(txParams); + delete populated.from; + return connected.signTransaction(populated); + } + + test("a populated, signed transaction is accepted and broadcastable", async () => { + const rawSignedTx = await popupSignsTx(TX_PARAMS); + const parsed = verifySignedTx(rawSignedTx, TX_PARAMS, signer.address); + expect(parsed.nonce).toBe(7); + expect(parsed.chainId).toBe(1n); + expect(parsed.gasLimit).toBe(21000n); + expect(parsed.to).toBe(RECIPIENT); + expect(parsed.value).toBe(BigInt(TX_PARAMS.value)); + expect(parsed.data).toBe(TX_PARAMS.data); + expect(parsed.signature).not.toBeNull(); + }); + + test("the wire payload carries no password and no secret", async () => { + const rawSignedTx = await popupSignsTx(TX_PARAMS); + const payload = { + type: "AUTISTMASK_TX_RESPONSE", + id: "test-approval-id", + approved: true, + rawSignedTx, + }; + expect(Object.keys(payload).sort()).toEqual([ + "approved", + "id", + "rawSignedTx", + "type", + ]); + const wire = JSON.stringify(payload).toLowerCase(); + expect(wire).not.toContain("password"); + expect(wire).not.toContain(SIGNER_KEY.slice(2).toLowerCase()); + }); + + test("the background rejects a transaction the popup did not approve", async () => { + const rawSignedTx = await popupSignsTx({ + ...TX_PARAMS, + to: OTHER_RECIPIENT, + }); + expect(() => + verifySignedTx(rawSignedTx, TX_PARAMS, signer.address), + ).toThrow(/approved recipient/); + }); +});