diff --git a/.gitignore b/.gitignore index fcc644e..81c1f4c 100644 --- 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 index bf3c25b..79a2945 100644 --- 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 index a4b8bf2..465064f 100644 --- 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 index a760307..310978d 100644 --- 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 index 70385b0..1e5889b 100644 --- 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 index 7985e37..aff3ba3 100644 --- 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', ); });