Files
AutistMask/tests/approvalVerify.test.js
sneak ab3b452924
All checks were successful
check / check (push) Successful in 28s
security: decrypt and sign dApp approvals in the popup (closes #157)
The dApp transaction and signature approval paths sent the user's plaintext
password to the background over runtime.sendMessage and decrypted there. Both
now decrypt in the popup, where the password is typed, and put only the signed
artifact on the wire: the raw signed transaction, or the signature. Neither the
password, the recovery phrase, the xprv nor the private key crosses the
messaging boundary any more. This matches what the popup-side eth_sendTransaction
path in confirmTx.js already did.

The popup runs the same sequence ethers' own sendTransaction() runs internally
(populateTransaction, then signTransaction), so nonce, gas, fee and chain id
population are unchanged. The background keeps broadcast and approval
resolution, and when the popup cannot produce an artifact it reports the error
over the same message so the requesting page still gets a failure rather than
hanging.

Moving the secret out of the background must not turn the background into a
blind relay, so it re-derives the signer from the artifact and checks it
against the approval it is holding before acting: shared/approvalVerify.js
asserts that a raw transaction is the approved transaction signed by the
approved address, and that a signature covers the approved payload and recovers
to the approved address.

A wrong password is now caught in the popup before anything is sent, so it
fails with an inline full-sentence error and leaves the pending approval alive
to retry; previously it reached the background and destroyed the approval.
Rejection still resolves with EIP-1193 code 4001, and approvals still survive
popup close and reopen.

Removes the four standing TODO(security) markers, now that the flaw is gone.
2026-08-09 02:50:50 +00:00

356 lines
12 KiB
JavaScript

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/);
});
});