fix: a listed site ending in a dot gets the phishing warning (closes #534)
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
This commit was merged in pull request #546.
This commit is contained in:
@@ -2663,7 +2663,8 @@ plaintext list of anyone's domain names.
|
||||
When a dApp on a blocklisted domain requests a wallet connection, transaction
|
||||
approval, or signature, the approval popup displays a prominent red warning
|
||||
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
|
||||
|
||||
|
||||
@@ -110,11 +110,14 @@ function hashListed(hash) {
|
||||
* Generate hostname variants for subdomain matching.
|
||||
* "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
|
||||
* @returns {string[]}
|
||||
*/
|
||||
function hostnameVariants(hostname) {
|
||||
const h = hostname.toLowerCase();
|
||||
const h = hostname.toLowerCase().replace(/\.$/, "");
|
||||
const variants = [h];
|
||||
const parts = h.split(".");
|
||||
// Parent domains: a.b.c.d -> b.c.d, c.d
|
||||
|
||||
@@ -89,6 +89,12 @@ describe("vendored blocklist", () => {
|
||||
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", () => {
|
||||
expect(isPhishingDomain("")).toBe(false);
|
||||
expect(isPhishingDomain(null)).toBe(false);
|
||||
@@ -133,6 +139,13 @@ describe("hostnameVariants", () => {
|
||||
test("lowercases hostnames", () => {
|
||||
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", () => {
|
||||
|
||||
Reference in New Issue
Block a user