fix: typed data with a missing field or a bool that is not true or false is refused (closes #552)

ethers signs a bool field by whether its value is truthy, so the string
"false" was shown as false and signed as true, and a bool field left out
was shown as nothing and signed as false. The sign screen now refuses
typed data whose message leaves out a declared field, at any level of
nesting, or holds a bool that is not true or false: the error line names
the field and Sign is disabled. The background refuses a signature for it
too. Presence is an own-key test, so a field named like a built-in
property such as constructor is not found by accident. signedPrimaryType()
moves to the shared module so the background reads the message as the
same type.

Model: opus-5-5

tests/backgroundApproval.test.js

@@ -384,13 +384,13 @@ function loadBackground(options) {
 
     // The same through eth_signTypedData_v4, whose params name the address
     // first and the typed data second.
-    function requestTypedData() {
+    function requestTypedData(typedData = TYPED_DATA) {
         let rpcResult = null;
         messageListener(
             {
                 type: "AUTISTMASK_RPC",
                 method: "eth_signTypedData_v4",
-                params: [signer.address, TYPED_DATA],
+                params: [signer.address, typedData],
             },
             { origin: ORIGIN },
             (r) => {
@@ -1494,8 +1494,8 @@ describe("what the approval is verified against", () => {
     // screen; a signature for it that reaches the background anyway is not
     // handed to the page (https://git.eeqj.de/sneak/AutistMask/issues/519).
     // TYPED_DATA names mainnet.
-    async function answerTypedData(bg, pending) {
-        const { domain, types, message } = JSON.parse(TYPED_DATA);
+    async function answerTypedData(bg, pending, typedData = TYPED_DATA) {
+        const { domain, types, message } = JSON.parse(typedData);
         delete types.EIP712Domain;
         const signature = await signer.signTypedData(domain, types, message);
         const answer = bg.send(
@@ -1539,6 +1539,42 @@ describe("what the approval is verified against", () => {
         expect(pending.result()).toEqual({ error: { message: refusal } });
     });
 
+    // ethers signs a `bool` field sent as the string "false" as true, and one
+    // left out as false. The sign screen refuses both; a signature for either
+    // that reaches the background anyway is not handed to the page
+    // (https://git.eeqj.de/sneak/AutistMask/issues/552).
+    test.each([
+        [
+            '"false"',
+            "false",
+            'The field "approved" is not true or false, so this typed data cannot be signed.',
+        ],
+        [
+            "left out",
+            undefined,
+            'The field "approved" is missing from the message, so this typed data cannot be signed.',
+        ],
+    ])(
+        "typed data with a bool %s is refused, not handed back",
+        async (_, approved, refusal) => {
+            const data = JSON.parse(TYPED_DATA);
+            data.types.Note.push({ name: "approved", type: "bool" });
+            data.message.approved = approved;
+            const typedData = JSON.stringify(data);
+            const bg = loadBackground();
+            const pending = bg.requestTypedData(typedData);
+            await settle();
+
+            const { answer } = await answerTypedData(bg, pending, typedData);
+
+            expect(answer.sendResponse).toHaveBeenCalledWith({
+                error: refusal,
+                retryable: false,
+            });
+            expect(pending.result()).toEqual({ error: { message: refusal } });
+        },
+    );
+
     test("a sign request from another address raises no approval at all", async () => {
         const bg = loadBackground();
         const pending = bg.requestSign(other.address);

tests/typedDataPermit.test.js

@@ -28,7 +28,7 @@ globalThis.chrome = {
     storage: { local: { get: async () => ({}), set: async () => {} } },
 };
 
-const { getAddress, MaxUint256 } = require("ethers");
+const { getAddress, MaxUint256, TypedDataEncoder } = require("ethers");
 const { state } = require("../src/shared/state");
 const { decryptWithPassword } = require("../src/shared/vault");
 const approval = require("../src/popup/views/approval");
@@ -662,3 +662,107 @@ describe("the sign screen refuses typed data for another chain", () => {
         expect(node("btn-approve-sign").disabled).toBe(false);
     });
 });
+
+// ethers signs a `bool` field by whether its value is truthy in JavaScript:
+// the string "false" is signed as true, and a field left out as false. The
+// screen would show one value while another is signed, so such typed data is
+// refused, and so is any declared field left out of the message
+// (https://git.eeqj.de/sneak/AutistMask/issues/552). DAI's older permit
+// declares the `bool` field `allowed`.
+describe("the sign screen refuses a message ethers would sign differently", () => {
+    const NOT_A_BOOL =
+        'The field "allowed" is not true or false, so this typed data cannot be signed.';
+    const MISSING =
+        'The field "allowed" is missing from the message, so this typed data cannot be signed.';
+
+    function daiPermit(allowed) {
+        return { ...DAI_PERMIT, message: { ...DAI_PERMIT.message, allowed } };
+    }
+
+    const withoutAllowed = {
+        ...DAI_PERMIT,
+        message: { ...DAI_PERMIT.message },
+    };
+    delete withoutAllowed.message.allowed;
+
+    beforeEach(() => {
+        decryptWithPassword.mockClear();
+    });
+
+    test('ethers signs "false" as true, and a bool left out as false', () => {
+        const hash = (data) =>
+            TypedDataEncoder.hash(
+                data.domain,
+                { Permit: data.types.Permit },
+                data.message,
+            );
+        expect(hash(daiPermit("false"))).toBe(hash(daiPermit(true)));
+        expect(hash(withoutAllowed)).toBe(hash(daiPermit(false)));
+    });
+
+    test('a bool sent as the string "false" shows the error, with Sign disabled', async () => {
+        await openSignScreen(daiPermit("false"));
+        expect(node("approve-sign-error").textContent).toBe(NOT_A_BOOL);
+        expect(node("btn-approve-sign").disabled).toBe(true);
+    });
+
+    test("a bool left out shows the error, with Sign disabled", async () => {
+        await openSignScreen(withoutAllowed);
+        expect(node("approve-sign-error").textContent).toBe(MISSING);
+        expect(node("btn-approve-sign").disabled).toBe(true);
+    });
+
+    test("Sign clicked anyway decrypts and signs nothing", async () => {
+        const sent = await openSignScreen(daiPermit("false"));
+        node("approve-sign-password").value = "any password";
+        await click("btn-approve-sign");
+
+        expect(decryptWithPassword).not.toHaveBeenCalled();
+        expect(sent.map((msg) => msg.type)).not.toContain(
+            "AUTISTMASK_SIGN_RESPONSE",
+        );
+        expect(node("approve-sign-error").textContent).toBe(NOT_A_BOOL);
+    });
+
+    test("a well-formed message leaves Sign enabled", async () => {
+        for (const allowed of [true, false]) {
+            await openSignScreen(daiPermit(allowed));
+            expect(node("approve-sign-error").textContent).toBe("");
+            expect(node("btn-approve-sign").disabled).toBe(false);
+        }
+    });
+
+    test("a field left out inside a list item is named by its place", () => {
+        const [first, second] = PERMIT_BATCH.message.details;
+        const withoutNonce = { ...second };
+        delete withoutNonce.nonce;
+        const data = {
+            ...PERMIT_BATCH,
+            message: {
+                ...PERMIT_BATCH.message,
+                details: [first, withoutNonce],
+            },
+        };
+        expect(typedDataRefusal(request(data))).toBe(
+            'The field "details[1].nonce" is missing from the message, so this typed data cannot be signed.',
+        );
+    });
+
+    // A plain lookup of a field named after a built-in object property reads
+    // back a function, which ethers would sign as true.
+    test("a field named after a built-in object property is missing when left out", () => {
+        const data = {
+            ...LOGIN,
+            types: {
+                ...LOGIN.types,
+                Login: [
+                    ...LOGIN.types.Login,
+                    { name: "constructor", type: "bool" },
+                ],
+            },
+        };
+        expect(typedDataRefusal(request(data))).toBe(
+            'The field "constructor" is missing from the message, so this typed data cannot be signed.',
+        );
+    });
+});