Compare commits

..
Author SHA1 Message Date
clawbot f1a13a50a6 fix: the defective xprv wallet notice names the keys that still reach its addresses (closes #255)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The notice told the user that these addresses are reachable by software
that repeats the same derivation, while the same wallet blocks Export
Private Key, so there was no way to take that route from here. The
second paragraph now names private key export among the disabled
controls and says the addresses can be reached outside this wallet with
the extended private key the user imported, if they kept a copy of it,
or with the master extended private key or recovery phrase it came
from. No behaviour changes. A test checks the new wording.

Model: opus-5-5
2026-10-14 11:48:46 +02:00
clawbot b4059af827 fix: a listed site ending in a dot gets the phishing warning (closes #534)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The blocklist check hashed the hostname as the page gave it, so
"myetheywallet.com." and every subdomain written that way hashed
differently from the listed "myetheywallet.com" and opened approval
prompts with no warning. The browser and DNS treat both spellings as
one host. hostnameVariants() now drops one trailing dot before
building the hostname and its parent domains, so the check sees the
name the list holds.

Model: opus-5-5
2026-10-14 10:48:41 +02:00
7 changed files with 44 additions and 130 deletions
+3 -7
View File
@@ -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
+4 -50
View File
@@ -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;
+4 -1
View File
@@ -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
+7 -5
View File
@@ -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 " +
+13
View File
@@ -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", () => {
+1 -67
View File
@@ -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.
+12
View File
@@ -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(" ");