diff --git a/README.md b/README.md
index ef7347b..c079601 100644
--- a/README.md
+++ b/README.md
@@ -2143,7 +2143,12 @@ view would leave a wallet one click from deletion.
Bytes that are not UTF-8 are shown as "This message is not text." For
typed data, formatted domain/type/message fields (EIP-712). The primary
type shown is the one ethers signs, derived from the typed data's `types`,
- not the type the site states.
+ not the type the site states. The message shows only the fields that type
+ declares, at every level of nesting and in the type's order: ethers signs
+ those and drops any other key. Typed data carrying other keys is not
+ refused; each such key is listed after the message under "Not part of the
+ signature", by its place in the message (`details.note`,
+ `details[1].note`) and its value.
- Token permission warning, at the top of the message (typed data whose
primary type is `Permit`, as in EIP-2612, or one of Permit2's signature
types): "⚠️ TOKEN PERMISSION: Signing this lets the spender below take the
diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js
index 823e465..2866129 100644
--- a/src/popup/views/approval.js
+++ b/src/popup/views/approval.js
@@ -626,10 +626,38 @@ function permitWarningHtml(types, primaryType, domain, message) {
return html;
}
+// What ethers signs of a message value of type `type`: a struct keeps only
+// the fields its type declares, in the type's order, and a list is read item
+// by item. ethers drops every other key, so each one is added to `notSigned`
+// with its place in the message and its value, for the screen to list apart.
+function signedValue(types, type, value, path, notSigned) {
+ const itemType = type.replace(/\[\d*\]$/, "");
+ if (itemType !== type) {
+ if (!Array.isArray(value)) return value;
+ return value.map((item, i) =>
+ signedValue(types, itemType, item, `${path}[${i}]`, notSigned),
+ );
+ }
+ const fields = types[type];
+ if (!fields || !value || typeof value !== "object") return value;
+ const at = (name) => (path ? `${path}.${name}` : name);
+ for (const [key, val] of Object.entries(value)) {
+ if (!fields.some((f) => f.name === key)) notSigned.push([at(key), val]);
+ }
+ return Object.fromEntries(
+ fields.map((f) => [
+ f.name,
+ signedValue(types, f.type, value[f.name], at(f.name), notSigned),
+ ]),
+ );
+}
+
// The typed data as the screen shows it. The primary type shown is the one
// ethers signs, never the page's word for it; typedDataRefusal() keeps the two
-// from differing on anything that can be signed. Only typed data that cannot
-// be read at all is shown as raw text, and typedDataRefusal() refuses it.
+// from differing on anything that can be signed. The Message section shows
+// only what is signed, and any other key the page sent is listed after it as
+// not part of the signature. Only typed data that cannot be read at all is
+// shown as raw text, and typedDataRefusal() refuses it.
function formatTypedDataHtml(jsonStr) {
let data;
let primaryType;
@@ -663,12 +691,30 @@ function formatTypedDataHtml(jsonStr) {
html += `
Primary type
`;
html += `
${escapeHtml(primaryType)}
`;
+ const lineHtml = (key, val) =>
+ `${escapeHtml(key)}: ${escapeHtml(display(val))}
`;
+
if (data.message) {
+ const notSigned = [];
+ const message = signedValue(
+ data.types,
+ primaryType,
+ data.message,
+ "",
+ notSigned,
+ );
html += `Message
`;
- for (const [key, val] of Object.entries(data.message)) {
- html += `
${escapeHtml(key)}: ${escapeHtml(display(val))}
`;
+ for (const [key, val] of Object.entries(message)) {
+ html += lineHtml(key, val);
}
html += `
`;
+ if (notSigned.length > 0) {
+ html += `Not part of the signature
`;
+ for (const [key, val] of notSigned) {
+ html += lineHtml(key, val);
+ }
+ html += `
`;
+ }
}
return html;
diff --git a/tests/typedDataPermit.test.js b/tests/typedDataPermit.test.js
index a9fcf96..3fb39ec 100644
--- a/tests/typedDataPermit.test.js
+++ b/tests/typedDataPermit.test.js
@@ -233,6 +233,26 @@ function warningOf(data) {
return html.slice(0, html.indexOf(">Domain<"));
}
+const NOT_SIGNED = ">Not part of the signature<";
+
+// The Message section alone, without the keys listed after it as not part of
+// the signature.
+function messageOf(data) {
+ const html = formatTypedDataHtml(JSON.stringify(data));
+ const end = html.indexOf(NOT_SIGNED);
+ return html.slice(
+ html.indexOf(">Message<"),
+ end === -1 ? html.length : end,
+ );
+}
+
+// The keys listed as not part of the signature, or "" when there are none.
+function notSignedOf(data) {
+ const html = formatTypedDataHtml(JSON.stringify(data));
+ const start = html.indexOf(NOT_SIGNED);
+ return start === -1 ? "" : html.slice(start);
+}
+
function request(data) {
return { method: "eth_signTypedData_v4", typedData: JSON.stringify(data) };
}
@@ -312,7 +332,8 @@ describe("the warning reads only the fields the signed type declares", () => {
});
// A Permit whose type names its spender `operator`; the page adds a
- // `spender` key the type does not declare.
+ // `spender` key the type does not declare. The Message section names
+ // only the operator, and the added key is listed apart as not signed.
test("an added key is not named as the spender", () => {
const data = {
...PERMIT,
@@ -331,6 +352,10 @@ describe("the warning reads only the fields the signed type declares", () => {
const warning = warningOf(data);
expect(warning).toContain(WARNING);
expect(warning).not.toContain(DECOY);
+ expect(messageOf(data)).toContain(SPENDER);
+ expect(messageOf(data)).not.toContain(DECOY);
+ expect(notSignedOf(data)).toContain("spender:");
+ expect(notSignedOf(data)).toContain(DECOY);
});
// The permit for a Uniswap v3 position NFT: a `Permit` with no amount
@@ -371,6 +396,47 @@ describe("the warning reads only the fields the signed type declares", () => {
});
});
+// ethers signs only the declared fields at every level of nesting, so an
+// added key inside a struct is not signed either. Such typed data is not
+// refused; the added key is listed apart, with where it sits in the message.
+describe("the Message section shows only the fields that are signed", () => {
+ const NOTE = "This signature only signs you in";
+
+ test("an added key inside a nested struct is listed as not signed", () => {
+ const data = {
+ ...PERMIT_SINGLE,
+ message: {
+ ...PERMIT_SINGLE.message,
+ details: { ...PERMIT_SINGLE.message.details, note: NOTE },
+ },
+ };
+ expect(messageOf(data)).toContain("expiration");
+ expect(messageOf(data)).not.toContain(NOTE);
+ expect(notSignedOf(data)).toContain("details.note:");
+ expect(notSignedOf(data)).toContain(NOTE);
+ expect(typedDataRefusal(request(data))).toBeNull();
+ });
+
+ test("an added key inside a list item is listed with its index", () => {
+ const [first, second] = PERMIT_BATCH.message.details;
+ const data = {
+ ...PERMIT_BATCH,
+ message: {
+ ...PERMIT_BATCH.message,
+ details: [first, { ...second, note: NOTE }],
+ },
+ };
+ expect(messageOf(data)).not.toContain(NOTE);
+ expect(notSignedOf(data)).toContain("details[1].note:");
+ expect(notSignedOf(data)).toContain(NOTE);
+ });
+
+ test("nothing is listed apart when every key is declared", () => {
+ expect(notSignedOf(PERMIT_SINGLE)).toBe("");
+ expect(notSignedOf(LOGIN)).toBe("");
+ });
+});
+
describe("the primary type shown is the one ethers signs", () => {
// A drain stated as a sign-in: the page says Login, the types say
// PermitSingle, and ethers would sign PermitSingle.