backport changes from latest (#129)

MetaMask · Dec 4, 2023 · 8441254 · 8441254
1 parent a375f98
commit 8441254
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 41 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,7 +1,7 @@
 .DS_Store
 dist/
 coverage/
-
+.vscode/
 # Logs
 logs
 *.log

diff --git a/src/index.ts b/src/index.ts
@@ -1,5 +1,5 @@
 import pump from 'pump';
-import { toASCII } from 'punycode/';
+import { toASCII, toUnicode } from 'punycode/';
 import PhishingDetector from 'eth-phishing-detect/src/detector';
 import { WindowPostMessageStream } from '@metamask/post-message-stream';
 import ObjectMultiplex from 'obj-multiplex';
@@ -138,6 +138,33 @@ async function isBlockedByMetamask(href: string) {
   }
 }
 
+/**
+ * Extract hostname and href from the query string.
+ *
+ * @returns The suspect hostname and href from the query string.
+ * @param href - The href value to check.
+ */
+function getSuspect(href = ''): {
+  suspectHostname: string;
+  suspectHostnameUnicode: string;
+  suspectHref: string;
+  suspectHrefUnicode: string;
+} {
+  try {
+    const url = new URL(href);
+    const unicodeHostname = toUnicode(url.hostname);
+    const unicodeHref = `${url.protocol}//${unicodeHostname}${url.pathname}${url.search}${url.hash}`;
+    return {
+      suspectHostname: url.hostname,
+      suspectHostnameUnicode: unicodeHostname,
+      suspectHref: url.href,
+      suspectHrefUnicode: unicodeHref,
+    };
+  } catch (error) {
+    throw new Error(`Invalid 'href' query parameter`);
+  }
+}
+
 /**
  * Initialize the phishing warning page streams.
  */
@@ -157,14 +184,13 @@ function start() {
   const { hash } = new URL(window.location.href);
   const hashContents = hash.slice(1); // drop leading '#' from hash
   const hashQueryString = new URLSearchParams(hashContents);
-  const suspectHostname = hashQueryString.get('hostname');
-  const suspectHref = hashQueryString.get('href');
 
-  if (!suspectHostname) {
-    throw new Error("Missing 'hostname' query parameter");
-  } else if (!suspectHref) {
-    throw new Error("Missing 'href' query parameter");
-  }
+  const {
+    suspectHostname,
+    suspectHref,
+    suspectHostnameUnicode,
+    suspectHrefUnicode,
+  } = getSuspect(hashQueryString.get('href'));
 
   const suspectLink = document.getElementById('suspect-link');
   if (!suspectLink) {
@@ -178,8 +204,8 @@ function start() {
   }
 
   const newIssueParams = `?title=[Legitimate%20Site%20Blocked]%20${encodeURIComponent(
-    suspectHostname,
-  )}&body=${encodeURIComponent(suspectHref)}`;
+    suspectHostnameUnicode,
+  )}&body=${encodeURIComponent(toUnicode(suspectHrefUnicode))}`;
 
   newIssueLink.addEventListener('click', async () => {
     const listName = (await isBlockedByMetamask(suspectHref))
@@ -194,7 +220,7 @@ function start() {
   }
 
   continueLink.addEventListener('click', async () => {
-    if (isValidSuspectHref(suspectHref) === false) {
+    if (!isValidSuspectHref(suspectHref)) {
       console.log(`Disallowed Protocol, cannot continue.`);
       return;
     }

diff --git a/tests/failed-lookup.spec.ts b/tests/failed-lookup.spec.ts
@@ -17,6 +17,6 @@ test('directs users to eth-phishing-detect to dispute a block, including issue t
   await page.waitForLoadState('networkidle');
 
   await expect(page).toHaveURL(
-    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com',
+    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com%2F',
   );
 });
diff --git a/tests/invalid-inputs.spec.ts b/tests/invalid-inputs.spec.ts
@@ -6,7 +6,7 @@ test.beforeEach(async ({ context }) => {
   await setupDefaultMocks(context);
 });
 
-test('throws an error about the hostname query parameter being missing', async ({
+test('throws an error about the href query parameter being invalid', async ({
   context,
   page,
 }) => {
@@ -16,29 +16,7 @@ test('throws an error about the hostname query parameter being missing', async (
       errorLogs.push(message.text());
     }
   });
-
-  await page.goto('/');
-
-  expect(errorLogs.length).toBe(1);
-  const browserName = context.browser()?.browserType().name();
-  expect(errorLogs[0]).toMatch(
-    browserName === 'firefox'
-      ? 'Error' // for some reason the message is missing on Firefox
-      : `Error: Missing 'hostname' query parameter`,
-  );
-});
-
-test('throws an error about the href query parameter being missing', async ({
-  context,
-  page,
-}) => {
-  const errorLogs: string[] = [];
-  page.on('console', (message) => {
-    if (message.type() === 'error') {
-      errorLogs.push(message.text());
-    }
-  });
-  const querystring = new URLSearchParams({ hostname: 'example.com' });
+  const querystring = new URLSearchParams({});
 
   await page.goto(`/#${querystring}`);
 
@@ -47,7 +25,7 @@ test('throws an error about the href query parameter being missing', async ({
   expect(errorLogs[0]).toMatch(
     browserName === 'firefox'
       ? 'Error' // for some reason the message is missing on Firefox
-      : `Error: Missing 'href' query parameter`,
+      : `Error: Invalid 'href' query parameter`,
   );
 });
 

diff --git a/tests/metamask-block.spec.ts b/tests/metamask-block.spec.ts
@@ -22,7 +22,7 @@ test('directs users to eth-phishing-detect to dispute a block, including issue t
   await page.waitForLoadState('networkidle');
 
   await expect(page).toHaveURL(
-    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com',
+    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com%2F',
   );
 });
 
@@ -44,6 +44,33 @@ test('correctly matches unicode domains', async ({ context, page }) => {
   await page.waitForLoadState('networkidle');
 
   await expect(page).toHaveURL(
-    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Fmetamạsk.io',
+    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20metam%E1%BA%A1sk.io&body=https%3A%2F%2Fmetam%E1%BA%A1sk.io%2F',
+  );
+});
+
+test('correctly matches unicode domains with path', async ({
+  context,
+  page,
+}) => {
+  await setupDefaultMocks(context, {
+    phishingConfig: {
+      ...defaultPhishingConfig,
+      blacklist: ['xn--metamsk-en4c.io'],
+    },
+  });
+  const url = 'https://metamạsk.io/somepath?query=string';
+  const querystring = new URLSearchParams({
+    hostname: url,
+    href: url,
+  });
+
+  await page.goto(`/#${querystring}`);
+
+  await page.getByRole('link', { name: 'report a detection problem' }).click();
+  // Wait for dynamic configuration check
+  await page.waitForLoadState('networkidle');
+
+  await expect(page).toHaveURL(
+    'https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20metam%E1%BA%A1sk.io&body=https%3A%2F%2Fmetam%E1%BA%A1sk.io%2Fsomepath%3Fquery%3Dstring',
   );
 });
diff --git a/tests/phishfort-block.spec.ts b/tests/phishfort-block.spec.ts
@@ -19,6 +19,6 @@ test('directs users to PhishFort to dispute a block, including issue template pa
   await page.waitForLoadState('networkidle');
 
   await expect(page).toHaveURL(
-    'https://github.com/phishfort/phishfort-lists/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com',
+    'https://github.com/phishfort/phishfort-lists/issues/new?title=[Legitimate%20Site%20Blocked]%20test.com&body=https%3A%2F%2Ftest.com%2F',
   );
 });