Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f1a13a50a6 | ||
|
|
b4059af827 |
@@ -2143,12 +2143,7 @@ view would leave a wallet one click from deletion.
|
|||||||
Bytes that are not UTF-8 are shown as "This message is not text." For
|
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
|
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`,
|
type shown is the one ethers signs, derived from the typed data's `types`,
|
||||||
not the type the site states. The message shows only the fields that type
|
not the type the site states.
|
||||||
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
|
- 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
|
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
|
types): "⚠️ TOKEN PERMISSION: Signing this lets the spender below take the
|
||||||
@@ -2668,7 +2663,8 @@ plaintext list of anyone's domain names.
|
|||||||
When a dApp on a blocklisted domain requests a wallet connection, transaction
|
When a dApp on a blocklisted domain requests a wallet connection, transaction
|
||||||
approval, or signature, the approval popup displays a prominent red warning
|
approval, or signature, the approval popup displays a prominent red warning
|
||||||
banner alerting the user. The domain checker matches exact hostnames and all
|
banner alerting the user. The domain checker matches exact hostnames and all
|
||||||
parent domains (subdomain matching).
|
parent domains (subdomain matching), and checks a hostname written with a
|
||||||
|
trailing dot (`name.`) as `name`.
|
||||||
|
|
||||||
#### Transaction Decoding
|
#### Transaction Decoding
|
||||||
|
|
||||||
|
|||||||
@@ -626,38 +626,10 @@ function permitWarningHtml(types, primaryType, domain, message) {
|
|||||||
return html;
|
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
|
// 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
|
// ethers signs, never the page's word for it; typedDataRefusal() keeps the two
|
||||||
// from differing on anything that can be signed. The Message section shows
|
// from differing on anything that can be signed. Only typed data that cannot
|
||||||
// only what is signed, and any other key the page sent is listed after it as
|
// be read at all is shown as raw text, and typedDataRefusal() refuses it.
|
||||||
// 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) {
|
function formatTypedDataHtml(jsonStr) {
|
||||||
let data;
|
let data;
|
||||||
let primaryType;
|
let primaryType;
|
||||||
@@ -691,30 +663,12 @@ function formatTypedDataHtml(jsonStr) {
|
|||||||
html += `<div class="mb-2"><div class="text-muted">Primary type</div>`;
|
html += `<div class="mb-2"><div class="text-muted">Primary type</div>`;
|
||||||
html += `<div class="font-bold">${escapeHtml(primaryType)}</div></div>`;
|
html += `<div class="font-bold">${escapeHtml(primaryType)}</div></div>`;
|
||||||
|
|
||||||
const lineHtml = (key, val) =>
|
|
||||||
`<div><span class="text-muted">${escapeHtml(key)}:</span> <span class="break-all">${escapeHtml(display(val))}</span></div>`;
|
|
||||||
|
|
||||||
if (data.message) {
|
if (data.message) {
|
||||||
const notSigned = [];
|
|
||||||
const message = signedValue(
|
|
||||||
data.types,
|
|
||||||
primaryType,
|
|
||||||
data.message,
|
|
||||||
"",
|
|
||||||
notSigned,
|
|
||||||
);
|
|
||||||
html += `<div class="mb-2"><div class="text-muted">Message</div>`;
|
html += `<div class="mb-2"><div class="text-muted">Message</div>`;
|
||||||
for (const [key, val] of Object.entries(message)) {
|
for (const [key, val] of Object.entries(data.message)) {
|
||||||
html += lineHtml(key, val);
|
html += `<div><span class="text-muted">${escapeHtml(key)}:</span> <span class="break-all">${escapeHtml(display(val))}</span></div>`;
|
||||||
}
|
}
|
||||||
html += `</div>`;
|
html += `</div>`;
|
||||||
if (notSigned.length > 0) {
|
|
||||||
html += `<div class="mb-2"><div class="text-muted">Not part of the signature</div>`;
|
|
||||||
for (const [key, val] of notSigned) {
|
|
||||||
html += lineHtml(key, val);
|
|
||||||
}
|
|
||||||
html += `</div>`;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return html;
|
return html;
|
||||||
|
|||||||
@@ -110,11 +110,14 @@ function hashListed(hash) {
|
|||||||
* Generate hostname variants for subdomain matching.
|
* Generate hostname variants for subdomain matching.
|
||||||
* "sub.evil.com" yields ["sub.evil.com", "evil.com"].
|
* "sub.evil.com" yields ["sub.evil.com", "evil.com"].
|
||||||
*
|
*
|
||||||
|
* A trailing dot is dropped: "evil.com." is the fully qualified spelling of
|
||||||
|
* "evil.com", and the browser loads the same site from either.
|
||||||
|
*
|
||||||
* @param {string} hostname
|
* @param {string} hostname
|
||||||
* @returns {string[]}
|
* @returns {string[]}
|
||||||
*/
|
*/
|
||||||
function hostnameVariants(hostname) {
|
function hostnameVariants(hostname) {
|
||||||
const h = hostname.toLowerCase();
|
const h = hostname.toLowerCase().replace(/\.$/, "");
|
||||||
const variants = [h];
|
const variants = [h];
|
||||||
const parts = h.split(".");
|
const parts = h.split(".");
|
||||||
// Parent domains: a.b.c.d -> b.c.d, c.d
|
// Parent domains: a.b.c.d -> b.c.d, c.d
|
||||||
|
|||||||
@@ -36,11 +36,13 @@ const DEFECTS = {
|
|||||||
"derivation path beneath that key instead of from a master " +
|
"derivation path beneath that key instead of from a master " +
|
||||||
"key, so the addresses listed here are not the ones that key " +
|
"key, so the addresses listed here are not the ones that key " +
|
||||||
"produces under the standard path.",
|
"produces under the standard path.",
|
||||||
"Signing and sending are disabled for this wallet. The addresses " +
|
"Signing, sending and private key export are disabled for this " +
|
||||||
"do descend from the extended private key you imported, so " +
|
"wallet. Anything these addresses hold can be reached outside " +
|
||||||
"anything they hold is still reachable by software that " +
|
"this wallet with the extended private key you imported, if " +
|
||||||
"repeats the same non-standard derivation. Check them in a " +
|
"you kept a copy of it, or with the master extended private " +
|
||||||
"block explorer before deciding what to do.",
|
"key or recovery phrase it came from, in software that " +
|
||||||
|
"repeats the same non-standard derivation. Check the " +
|
||||||
|
"addresses in a block explorer before deciding what to do.",
|
||||||
"To see the addresses this key produces under the standard path, " +
|
"To see the addresses this key produces under the standard path, " +
|
||||||
"import the master extended private key, or the recovery " +
|
"import the master extended private key, or the recovery " +
|
||||||
"phrase it came from, as a new wallet. Nothing here has been " +
|
"phrase it came from, as a new wallet. Nothing here has been " +
|
||||||
|
|||||||
@@ -89,6 +89,12 @@ describe("vendored blocklist", () => {
|
|||||||
expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
|
expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a hostname ending in a dot is the same host", () => {
|
||||||
|
expect(isPhishingDomain(LISTED[1] + ".")).toBe(true);
|
||||||
|
expect(isPhishingDomain("wallet." + LISTED[1] + ".")).toBe(true);
|
||||||
|
expect(isPhishingDomain(CLEAN[0] + ".")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
test("returns false for an empty or missing hostname", () => {
|
test("returns false for an empty or missing hostname", () => {
|
||||||
expect(isPhishingDomain("")).toBe(false);
|
expect(isPhishingDomain("")).toBe(false);
|
||||||
expect(isPhishingDomain(null)).toBe(false);
|
expect(isPhishingDomain(null)).toBe(false);
|
||||||
@@ -133,6 +139,13 @@ describe("hostnameVariants", () => {
|
|||||||
test("lowercases hostnames", () => {
|
test("lowercases hostnames", () => {
|
||||||
expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
|
expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("drops the trailing dot of a fully qualified hostname", () => {
|
||||||
|
expect(hostnameVariants("sub.evil.com.")).toEqual([
|
||||||
|
"sub.evil.com",
|
||||||
|
"evil.com",
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("domain hashing", () => {
|
describe("domain hashing", () => {
|
||||||
|
|||||||
@@ -233,26 +233,6 @@ function warningOf(data) {
|
|||||||
return html.slice(0, html.indexOf(">Domain<"));
|
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) {
|
function request(data) {
|
||||||
return { method: "eth_signTypedData_v4", typedData: JSON.stringify(data) };
|
return { method: "eth_signTypedData_v4", typedData: JSON.stringify(data) };
|
||||||
}
|
}
|
||||||
@@ -332,8 +312,7 @@ describe("the warning reads only the fields the signed type declares", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// A Permit whose type names its spender `operator`; the page adds a
|
// A Permit whose type names its spender `operator`; the page adds a
|
||||||
// `spender` key the type does not declare. The Message section names
|
// `spender` key the type does not declare.
|
||||||
// only the operator, and the added key is listed apart as not signed.
|
|
||||||
test("an added key is not named as the spender", () => {
|
test("an added key is not named as the spender", () => {
|
||||||
const data = {
|
const data = {
|
||||||
...PERMIT,
|
...PERMIT,
|
||||||
@@ -352,10 +331,6 @@ describe("the warning reads only the fields the signed type declares", () => {
|
|||||||
const warning = warningOf(data);
|
const warning = warningOf(data);
|
||||||
expect(warning).toContain(WARNING);
|
expect(warning).toContain(WARNING);
|
||||||
expect(warning).not.toContain(DECOY);
|
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
|
// The permit for a Uniswap v3 position NFT: a `Permit` with no amount
|
||||||
@@ -396,47 +371,6 @@ 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", () => {
|
describe("the primary type shown is the one ethers signs", () => {
|
||||||
// A drain stated as a sign-in: the page says Login, the types say
|
// A drain stated as a sign-in: the page says Login, the types say
|
||||||
// PermitSingle, and ethers would sign PermitSingle.
|
// PermitSingle, and ethers would sign PermitSingle.
|
||||||
|
|||||||
@@ -153,6 +153,18 @@ describe("the explanatory copy", () => {
|
|||||||
expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
|
expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The wallet holds the imported key but will not export it, so the copy
|
||||||
|
// must name every key the user may hold that reaches the addresses
|
||||||
|
// outside it.
|
||||||
|
test("it names the keys that reach the addresses outside this wallet", () => {
|
||||||
|
const body = defect.paragraphs.join(" ");
|
||||||
|
|
||||||
|
expect(body).toMatch(/private key export are disabled/);
|
||||||
|
expect(body).toMatch(
|
||||||
|
/with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test("it says the wallet is not deleted and what the user can do", () => {
|
test("it says the wallet is not deleted and what the user can do", () => {
|
||||||
const body = defect.paragraphs.join(" ");
|
const body = defect.paragraphs.join(" ");
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user