From 7c39264e484f15b0c9fe02a955e395af1141759a Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Tue, 29 Oct 2024 22:47:38 -0400 Subject: [PATCH 1/6] fix: mobile perf issue --- .../src/PhishingController.ts | 20 ++ .../src/PhishingDetector.ts | 179 ++++++++++++++---- packages/phishing-controller/src/types.ts | 5 + 3 files changed, 172 insertions(+), 32 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index cdf8eaab23..1952a66130 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -489,6 +489,26 @@ export class PhishingController extends BaseController< return this.#detector.isMaliciousC2Domain(punycodeOrigin); } + /** + * Determines if a given origin is unapproved or malicious. + * + * @param origin - Domain origin of a website. + * @returns A promise that resolves to the phishing detection result. + */ + async scanDomain(origin: string): Promise { + const punycodeOrigin = toASCII(origin); + const hostname = getHostnameFromUrl(punycodeOrigin); + + if (this.state.whitelist.includes(hostname || punycodeOrigin)) { + return { + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }; + } + + return this.#detector.scanDomain(punycodeOrigin); + } + /** * Temporarily marks a given origin as approved. * diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index 3cb35e780f..ea4154e571 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -16,6 +16,12 @@ import { sha256Hash, } from './utils'; +const DAPP_SCAN_API_BASE_URL = 'https://dapp-scanning.api.cx.metamask.io'; +const DAPP_SCAN_ENDPOINT = '/scan'; +const DAPP_SCAN_REQUEST_TIMEOUT = 5000; // 5 seconds in milliseconds + +// Phishing Detector Types + export type LegacyPhishingDetectorList = { whitelist?: string[]; blacklist?: string[]; @@ -55,6 +61,18 @@ export type PhishingDetectorConfiguration = { tolerance: number; }; +export type DappScanResponse = { + domainName: string; + recommendedAction: 'BLOCK' | 'WARN' | 'NONE'; + riskFactors: { + type: string; + severity: string; + message: string; + }[]; + verified: boolean; + status: string; +}; + export class PhishingDetector { #configs: PhishingDetectorConfiguration[]; @@ -154,29 +172,18 @@ export class PhishingDetector { }; } - const fqdn = domain.endsWith('.') ? domain.slice(0, -1) : domain; - - const source = domainToParts(fqdn); + const fqdn = this.#normalizeDomain(domain); + const sourceParts = domainToParts(fqdn); - for (const { allowlist, name, version } of this.#configs) { - // if source matches allowlist hostname (or subdomain thereof), PASS - const allowlistMatch = matchPartsAgainstList(source, allowlist); - if (allowlistMatch) { - const match = domainPartsToDomain(allowlistMatch); - return { - match, - name, - result: false, - type: PhishingDetectorResultType.Allowlist, - version: version === undefined ? version : String(version), - }; - } + const allowlistResult = this.#checkAllowlist(sourceParts); + if (allowlistResult) { + return allowlistResult; } for (const { blocklist, fuzzylist, name, tolerance, version } of this .#configs) { // if source matches blocklist hostname (or subdomain thereof), FAIL - const blocklistMatch = matchPartsAgainstList(source, blocklist); + const blocklistMatch = matchPartsAgainstList(sourceParts, blocklist); if (blocklistMatch) { const match = domainPartsToDomain(blocklistMatch); return { @@ -190,7 +197,7 @@ export class PhishingDetector { if (tolerance > 0) { // check if near-match of whitelist domain, FAIL - let fuzzyForm = domainPartsToFuzzyForm(source); + let fuzzyForm = domainPartsToFuzzyForm(sourceParts); // strip www fuzzyForm = fuzzyForm.replace(/^www\./u, ''); // check against fuzzylist @@ -233,22 +240,12 @@ export class PhishingDetector { }; } - const fqdn = hostname.endsWith('.') ? hostname.slice(0, -1) : hostname; + const fqdn = this.#normalizeDomain(hostname); const sourceParts = domainToParts(fqdn); - for (const { allowlist, name, version } of this.#configs) { - // if source matches allowlist hostname (or subdomain thereof), PASS - const allowlistMatch = matchPartsAgainstList(sourceParts, allowlist); - if (allowlistMatch) { - const match = domainPartsToDomain(allowlistMatch); - return { - match, - name, - result: false, - type: PhishingDetectorResultType.Allowlist, - version: version === undefined ? version : String(version), - }; - } + const allowlistResult = this.#checkAllowlist(sourceParts); + if (allowlistResult) { + return allowlistResult; } const hostnameHash = sha256Hash(hostname.toLowerCase()); @@ -286,6 +283,124 @@ export class PhishingDetector { type: PhishingDetectorResultType.C2DomainBlocklist, }; } + + async scanDomain(punycodeOrigin: string): Promise { + const fqdn = this.#normalizeDomain(punycodeOrigin); + const sourceParts = domainToParts(fqdn); + + const allowlistResult = this.#checkAllowlist(sourceParts); + if (allowlistResult) { + return allowlistResult; + } + + return await this.#fetchDappScanResult(fqdn); + } + + /** + * Fetches the dApp scan result from the external API. + * + * @param fqdn - The fully qualified domain name to scan. + * @returns A PhishingDetectorResult based on the API response. + */ + async #fetchDappScanResult(fqdn: string): Promise { + const apiUrl = `${DAPP_SCAN_API_BASE_URL}${DAPP_SCAN_ENDPOINT}?url=${fqdn}`; + + try { + const response = await this.#fetchWithTimeout( + apiUrl, + DAPP_SCAN_REQUEST_TIMEOUT, + ); + + if (!response.ok) { + console.error( + `dApp Scan API error: ${response.status} ${response.statusText}`, + ); + return { + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }; + } + + const data: DappScanResponse = await response.json(); + + if (data.recommendedAction === 'BLOCK') { + return { + result: true, + type: PhishingDetectorResultType.RealTimeDappScan, + name: 'DappScan', + version: '1', + match: fqdn, + }; + } + + return { + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }; + } catch (error) { + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + console.error(`dApp Scan fetch error: ${error}`); + return { + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }; + } + } + + /** + * Checks if the domain is in the allowlist. + * + * @param sourceParts - The parts of the domain. + * @returns A PhishingDetectorResult if matched; otherwise, undefined. + */ + #checkAllowlist(sourceParts: string[]): PhishingDetectorResult | undefined { + for (const { allowlist, name, version } of this.#configs) { + const allowlistMatch = matchPartsAgainstList(sourceParts, allowlist); + if (allowlistMatch) { + const match = domainPartsToDomain(allowlistMatch); + return { + match, + name, + result: false, + type: PhishingDetectorResultType.Allowlist, + version: version === undefined ? version : String(version), + }; + } + } + return undefined; + } + + /** + * Normalizes the domain by removing any trailing dot. + * + * @param domain - The domain to normalize. + * @returns The normalized domain. + */ + #normalizeDomain(domain: string): string { + return domain.endsWith('.') ? domain.slice(0, -1) : domain; + } + + /** + * Fetch with a timeout. + * + * @param url - The URL to fetch. + * @param timeout - The timeout in milliseconds. + * @returns A promise that resolves to the fetch Response. + */ + async #fetchWithTimeout(url: string, timeout: number): Promise { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeout); + + try { + const response = await fetch(url, { + signal: controller.signal, + cache: 'no-cache', + }); + return response; + } finally { + clearTimeout(timeoutId); + } + } } /** diff --git a/packages/phishing-controller/src/types.ts b/packages/phishing-controller/src/types.ts index 2e0bb2c76c..dac826d30a 100644 --- a/packages/phishing-controller/src/types.ts +++ b/packages/phishing-controller/src/types.ts @@ -70,4 +70,9 @@ export enum PhishingDetectorResultType { * "c2DomainBlocklist" means that the domain was found in the C2 domain blocklist. */ C2DomainBlocklist = 'c2DomainBlocklist', + + /* + * "realTimeDappScan" means that the domain was found in the realTimeDappScan list. + */ + RealTimeDappScan = 'realTimeDappScan', } From 9a2d054180bcbf08f627b9a01f881138d7cc9b8d Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Tue, 29 Oct 2024 22:50:46 -0400 Subject: [PATCH 2/6] chore: clean up comments --- packages/phishing-controller/src/PhishingDetector.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index ea4154e571..253fb37b78 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -20,8 +20,6 @@ const DAPP_SCAN_API_BASE_URL = 'https://dapp-scanning.api.cx.metamask.io'; const DAPP_SCAN_ENDPOINT = '/scan'; const DAPP_SCAN_REQUEST_TIMEOUT = 5000; // 5 seconds in milliseconds -// Phishing Detector Types - export type LegacyPhishingDetectorList = { whitelist?: string[]; blacklist?: string[]; From 0a800cac612a7ad5866c350eceb5e6ba14c61811 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Tue, 29 Oct 2024 22:54:29 -0400 Subject: [PATCH 3/6] chore: cleanup functions --- .../src/PhishingDetector.ts | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index 253fb37b78..c8abd63a40 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -282,25 +282,50 @@ export class PhishingDetector { }; } + /** + * Scans a domain to determine if it is malicious by: + * 1. Checking against the allowlist, and if found, returning a safe result. + * 2. Fetching data from the dApp scan API to analyze risk. + * 3. Checking if the API recommends blocking the domain based on its risk profile. + * + * @param punycodeOrigin - The punycode-encoded domain to scan. + * @returns A PhishingDetectorResult indicating if the domain is safe or malicious. + */ async scanDomain(punycodeOrigin: string): Promise { const fqdn = this.#normalizeDomain(punycodeOrigin); - const sourceParts = domainToParts(fqdn); + const sourceParts = domainToParts(fqdn); const allowlistResult = this.#checkAllowlist(sourceParts); if (allowlistResult) { return allowlistResult; } - return await this.#fetchDappScanResult(fqdn); + const data = await this.#fetchDappScanResult(fqdn); + + if (data && data.recommendedAction === 'BLOCK') { + return { + result: true, + type: PhishingDetectorResultType.RealTimeDappScan, + name: 'DappScan', + version: '1', + match: fqdn, + }; + } + + // Otherwise, return a safe result + return { + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }; } /** - * Fetches the dApp scan result from the external API. + * Fetches the raw dApp scan result data from the external API. * * @param fqdn - The fully qualified domain name to scan. - * @returns A PhishingDetectorResult based on the API response. + * @returns The raw data from the dApp scan API or null if the request fails. */ - async #fetchDappScanResult(fqdn: string): Promise { + async #fetchDappScanResult(fqdn: string): Promise { const apiUrl = `${DAPP_SCAN_API_BASE_URL}${DAPP_SCAN_ENDPOINT}?url=${fqdn}`; try { @@ -313,35 +338,15 @@ export class PhishingDetector { console.error( `dApp Scan API error: ${response.status} ${response.statusText}`, ); - return { - result: false, - type: PhishingDetectorResultType.RealTimeDappScan, - }; + return null; } const data: DappScanResponse = await response.json(); - - if (data.recommendedAction === 'BLOCK') { - return { - result: true, - type: PhishingDetectorResultType.RealTimeDappScan, - name: 'DappScan', - version: '1', - match: fqdn, - }; - } - - return { - result: false, - type: PhishingDetectorResultType.RealTimeDappScan, - }; + return data; } catch (error) { // eslint-disable-next-line @typescript-eslint/restrict-template-expressions console.error(`dApp Scan fetch error: ${error}`); - return { - result: false, - type: PhishingDetectorResultType.RealTimeDappScan, - }; + return null; } } From cbd01a33fd1676af820fdbe829e1bc0bdf23cd27 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Tue, 29 Oct 2024 22:59:04 -0400 Subject: [PATCH 4/6] chore: add RecommendedAction type --- .../src/PhishingDetector.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index c8abd63a40..079ede7ae1 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -61,7 +61,7 @@ export type PhishingDetectorConfiguration = { export type DappScanResponse = { domainName: string; - recommendedAction: 'BLOCK' | 'WARN' | 'NONE'; + recommendedAction: RecommendedAction; riskFactors: { type: string; severity: string; @@ -71,6 +71,20 @@ export type DappScanResponse = { status: string; }; +/** + * RecommendedAction represents the warning type based on the risk factors. + */ +type RecommendedAction = 'NONE' | 'WARN' | 'BLOCK'; + +/** + * Enum-like object to provide named constants for RecommendedAction values. + */ +const RecommendedAction = { + None: 'NONE' as RecommendedAction, + Warn: 'WARN' as RecommendedAction, + Block: 'BLOCK' as RecommendedAction, +}; + export class PhishingDetector { #configs: PhishingDetectorConfiguration[]; @@ -302,7 +316,7 @@ export class PhishingDetector { const data = await this.#fetchDappScanResult(fqdn); - if (data && data.recommendedAction === 'BLOCK') { + if (data && data.recommendedAction === RecommendedAction.Block) { return { result: true, type: PhishingDetectorResultType.RealTimeDappScan, From dba51207d141af3bc2f19e030ddec1435cc5ed8e Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Tue, 29 Oct 2024 23:05:43 -0400 Subject: [PATCH 5/6] chore: cleanup comments --- .../phishing-controller/src/PhishingDetector.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index 079ede7ae1..370b0ac06a 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -71,14 +71,8 @@ export type DappScanResponse = { status: string; }; -/** - * RecommendedAction represents the warning type based on the risk factors. - */ type RecommendedAction = 'NONE' | 'WARN' | 'BLOCK'; -/** - * Enum-like object to provide named constants for RecommendedAction values. - */ const RecommendedAction = { None: 'NONE' as RecommendedAction, Warn: 'WARN' as RecommendedAction, @@ -297,10 +291,7 @@ export class PhishingDetector { } /** - * Scans a domain to determine if it is malicious by: - * 1. Checking against the allowlist, and if found, returning a safe result. - * 2. Fetching data from the dApp scan API to analyze risk. - * 3. Checking if the API recommends blocking the domain based on its risk profile. + * Scans a domain to determine if it is malicious * * @param punycodeOrigin - The punycode-encoded domain to scan. * @returns A PhishingDetectorResult indicating if the domain is safe or malicious. @@ -334,10 +325,10 @@ export class PhishingDetector { } /** - * Fetches the raw dApp scan result data from the external API. + * Fetches the dApp scan result data from the dapp scanning API. * * @param fqdn - The fully qualified domain name to scan. - * @returns The raw data from the dApp scan API or null if the request fails. + * @returns The data from the dApp scan API or null if the request fails. */ async #fetchDappScanResult(fqdn: string): Promise { const apiUrl = `${DAPP_SCAN_API_BASE_URL}${DAPP_SCAN_ENDPOINT}?url=${fqdn}`; From 91736db72e78670ffc995d16dbb80c5e24ae6702 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Wed, 30 Oct 2024 01:07:49 -0400 Subject: [PATCH 6/6] chore: 90% test coverage --- .../src/PhishingController.test.ts | 104 ++++++++ .../src/PhishingDetector.test.ts | 232 ++++++++++++++++++ .../src/PhishingDetector.ts | 53 +--- .../phishing-controller/src/utils.test.ts | 34 +++ packages/phishing-controller/src/utils.ts | 35 +++ 5 files changed, 417 insertions(+), 41 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 810f7a179a..d9072e45ad 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -14,6 +14,12 @@ import { CLIENT_SIDE_DETECION_BASE_URL, C2_DOMAIN_BLOCKLIST_ENDPOINT, } from './PhishingController'; +import type { DappScanResponse } from './PhishingDetector'; +import { + DAPP_SCAN_API_BASE_URL, + DAPP_SCAN_ENDPOINT, + RecommendedAction, +} from './PhishingDetector'; import { formatHostnameToUrl } from './tests/utils'; import { PhishingDetectorResultType } from './types'; import { getHostnameFromUrl } from './utils'; @@ -2370,4 +2376,102 @@ describe('PhishingController', () => { expect(controller.state.whitelist).toHaveLength(1); }); }); + describe('PhishingController - scanDomain', () => { + afterEach(() => { + nock.cleanAll(); + }); + + it('should return safe if domain is in the allowlist', async () => { + const allowlistedDomain = 'example.com'; + + const controller = getPhishingController(); + + const result = await controller.scanDomain(allowlistedDomain); + expect(result).toMatchObject({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }); + + it('should return false if the URL is in the whitelist', async () => { + const whitelistedHostname = 'example.com'; + + const controller = getPhishingController(); + controller.bypass(formatHostnameToUrl(whitelistedHostname)); + const result = await controller.scanDomain(whitelistedHostname); + expect(result).toMatchObject({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }); + + it('should return malicious if the dApp scan API recommends BLOCK', async () => { + const domainToScan = 'malicious.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domainToScan}`) + .reply(200, { + domainName: domainToScan, + recommendedAction: RecommendedAction.Block, + riskFactors: [ + { + type: 'DRAINER', + severity: 'CRITICAL', + message: 'Domain identified as a wallet drainer.', + }, + ], + verified: false, + status: 'COMPLETE', + } as DappScanResponse); + + const controller = getPhishingController(); + + const result = await controller.scanDomain(domainToScan); + expect(result).toStrictEqual({ + result: true, + type: PhishingDetectorResultType.RealTimeDappScan, + name: 'DappScan', + version: '1', + match: domainToScan, + }); + }); + + it('should return safe if the dApp scan API recommends NONE', async () => { + const domainToScan = 'none.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domainToScan}`) + .reply(200, { + domainName: domainToScan, + recommendedAction: RecommendedAction.None, + riskFactors: [], + verified: true, + status: 'COMPLETE', + } as DappScanResponse); + + const controller = getPhishingController(); + + const result = await controller.scanDomain(domainToScan); + expect(result).toMatchObject({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }); + + it('should return safe if the dApp scan API request fails', async () => { + const domainToScan = 'unreachable.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domainToScan}`) + .replyWithError('Network error'); + + const controller = getPhishingController(); + + const result = await controller.scanDomain(domainToScan); + expect(result).toMatchObject({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }); + }); }); diff --git a/packages/phishing-controller/src/PhishingDetector.test.ts b/packages/phishing-controller/src/PhishingDetector.test.ts index f76b809965..754253bb88 100644 --- a/packages/phishing-controller/src/PhishingDetector.test.ts +++ b/packages/phishing-controller/src/PhishingDetector.test.ts @@ -1,4 +1,8 @@ +import nock from 'nock'; + import { + DAPP_SCAN_API_BASE_URL, + DAPP_SCAN_ENDPOINT, PhishingDetector, type PhishingDetectorOptions, } from './PhishingDetector'; @@ -1782,6 +1786,234 @@ describe('PhishingDetector', () => { }, ); }); + + describe('scanDomain', () => { + it('should return allowlist result if domain is in the allowlist', async () => { + const allowlistedDomain = 'example.com'; + + await withPhishingDetector( + [ + { + allowlist: ['example.com'], + blocklist: [], + fuzzylist: [], + name: 'allowlist-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(allowlistedDomain); + expect(result).toStrictEqual({ + match: 'example.com', + name: 'allowlist-config', + result: false, + type: PhishingDetectorResultType.Allowlist, + version: '1', + }); + }, + ); + }); + + it('should return malicious result if dApp scan API recommends BLOCK', async () => { + const maliciousDomain = 'malicious.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${maliciousDomain}`) + .reply(200, { + domainName: maliciousDomain, + recommendedAction: 'BLOCK', + riskFactors: [ + { + type: 'DRAINER', + severity: 'CRITICAL', + message: 'Domain identified as a wallet drainer.', + }, + ], + verified: false, + status: 'COMPLETE', + }); + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: [], + fuzzylist: [], + name: 'api-block-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(maliciousDomain); + expect(result).toStrictEqual({ + result: true, + type: PhishingDetectorResultType.RealTimeDappScan, + name: 'DappScan', + version: '1', + match: maliciousDomain, + }); + }, + ); + + expect(nock.isDone()).toBe(true); + }); + + it('should return safe result if dApp scan API does not recommend BLOCK', async () => { + const safeDomain = 'safe.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${safeDomain}`) + .reply(200, { + domainName: safeDomain, + recommendedAction: 'WARN', + riskFactors: [ + { + type: 'SUSPICIOUS', + severity: 'MEDIUM', + message: 'Domain has suspicious activity.', + }, + ], + verified: true, + status: 'COMPLETE', + }); + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: [], + fuzzylist: [], + name: 'api-warn-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(safeDomain); + expect(result).toStrictEqual({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }, + ); + + expect(nock.isDone()).toBe(true); + }); + + it('should return safe result if dApp scan API responds with non-OK status', async () => { + const domain = 'unknown.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domain}`) + .reply(500, 'Internal Server Error'); + + const consoleErrorSpy = jest.spyOn(console, 'error'); + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: [], + fuzzylist: [], + name: 'api-error-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(domain); + expect(result).toStrictEqual({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }, + ); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'dApp Scan API error: 500 Internal Server Error', + ); + + consoleErrorSpy.mockRestore(); + + expect(nock.isDone()).toBe(true); + }); + + it('should return safe result if dApp scan API request fails due to network error', async () => { + const domain = 'network-error.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domain}`) + .replyWithError('Network Failure'); + + const consoleErrorSpy = jest.spyOn(console, 'error'); + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: [], + fuzzylist: [], + name: 'api-network-error-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(domain); + expect(result).toStrictEqual({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }, + ); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('dApp Scan fetch error:'), + ); + + consoleErrorSpy.mockRestore(); + + expect(nock.isDone()).toBe(true); + }); + + it('should return safe result if domain is not in allowlist and API does not recommend BLOCK', async () => { + const domain = 'neutral.com'; + + nock(DAPP_SCAN_API_BASE_URL) + .get(`${DAPP_SCAN_ENDPOINT}?url=${domain}`) + .reply(200, { + domainName: domain, + recommendedAction: 'NONE', + riskFactors: [], + verified: true, + status: 'COMPLETE', + }); + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: [], + fuzzylist: [], + name: 'api-none-config', + version: 1, + tolerance: 2, + }, + ], + async ({ detector }) => { + const result = await detector.scanDomain(domain); + expect(result).toStrictEqual({ + result: false, + type: PhishingDetectorResultType.RealTimeDappScan, + }); + }, + ); + + expect(nock.isDone()).toBe(true); + }); + }); }); type WithPhishingDetectorCallback = ({ diff --git a/packages/phishing-controller/src/PhishingDetector.ts b/packages/phishing-controller/src/PhishingDetector.ts index 370b0ac06a..47c4c3049b 100644 --- a/packages/phishing-controller/src/PhishingDetector.ts +++ b/packages/phishing-controller/src/PhishingDetector.ts @@ -8,17 +8,20 @@ import { domainPartsToDomain, domainPartsToFuzzyForm, domainToParts, + fetchWithTimeout, generateParentDomains, getDefaultPhishingDetectorConfig, getHostnameFromUrl, matchPartsAgainstList, + normalizeDomain, processConfigs, sha256Hash, } from './utils'; -const DAPP_SCAN_API_BASE_URL = 'https://dapp-scanning.api.cx.metamask.io'; -const DAPP_SCAN_ENDPOINT = '/scan'; -const DAPP_SCAN_REQUEST_TIMEOUT = 5000; // 5 seconds in milliseconds +export const DAPP_SCAN_API_BASE_URL = + 'https://dapp-scanning.api.cx.metamask.io'; +export const DAPP_SCAN_ENDPOINT = '/scan'; +export const DAPP_SCAN_REQUEST_TIMEOUT = 5000; // 5 seconds in milliseconds export type LegacyPhishingDetectorList = { whitelist?: string[]; @@ -71,9 +74,9 @@ export type DappScanResponse = { status: string; }; -type RecommendedAction = 'NONE' | 'WARN' | 'BLOCK'; +export type RecommendedAction = 'NONE' | 'WARN' | 'BLOCK'; -const RecommendedAction = { +export const RecommendedAction = { None: 'NONE' as RecommendedAction, Warn: 'WARN' as RecommendedAction, Block: 'BLOCK' as RecommendedAction, @@ -178,7 +181,7 @@ export class PhishingDetector { }; } - const fqdn = this.#normalizeDomain(domain); + const fqdn = normalizeDomain(domain); const sourceParts = domainToParts(fqdn); const allowlistResult = this.#checkAllowlist(sourceParts); @@ -246,7 +249,7 @@ export class PhishingDetector { }; } - const fqdn = this.#normalizeDomain(hostname); + const fqdn = normalizeDomain(hostname); const sourceParts = domainToParts(fqdn); const allowlistResult = this.#checkAllowlist(sourceParts); @@ -297,7 +300,7 @@ export class PhishingDetector { * @returns A PhishingDetectorResult indicating if the domain is safe or malicious. */ async scanDomain(punycodeOrigin: string): Promise { - const fqdn = this.#normalizeDomain(punycodeOrigin); + const fqdn = normalizeDomain(punycodeOrigin); const sourceParts = domainToParts(fqdn); const allowlistResult = this.#checkAllowlist(sourceParts); @@ -334,7 +337,7 @@ export class PhishingDetector { const apiUrl = `${DAPP_SCAN_API_BASE_URL}${DAPP_SCAN_ENDPOINT}?url=${fqdn}`; try { - const response = await this.#fetchWithTimeout( + const response = await fetchWithTimeout( apiUrl, DAPP_SCAN_REQUEST_TIMEOUT, ); @@ -377,38 +380,6 @@ export class PhishingDetector { } return undefined; } - - /** - * Normalizes the domain by removing any trailing dot. - * - * @param domain - The domain to normalize. - * @returns The normalized domain. - */ - #normalizeDomain(domain: string): string { - return domain.endsWith('.') ? domain.slice(0, -1) : domain; - } - - /** - * Fetch with a timeout. - * - * @param url - The URL to fetch. - * @param timeout - The timeout in milliseconds. - * @returns A promise that resolves to the fetch Response. - */ - async #fetchWithTimeout(url: string, timeout: number): Promise { - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), timeout); - - try { - const response = await fetch(url, { - signal: controller.signal, - cache: 'no-cache', - }); - return response; - } finally { - clearTimeout(timeoutId); - } - } } /** diff --git a/packages/phishing-controller/src/utils.test.ts b/packages/phishing-controller/src/utils.test.ts index b73cb572af..94caca153a 100644 --- a/packages/phishing-controller/src/utils.test.ts +++ b/packages/phishing-controller/src/utils.test.ts @@ -1,3 +1,4 @@ +import nock from 'nock'; import * as sinon from 'sinon'; import { ListKeys, ListNames } from './PhishingController'; @@ -5,9 +6,11 @@ import { applyDiffs, domainToParts, fetchTimeNow, + fetchWithTimeout, generateParentDomains, getHostnameFromUrl, matchPartsAgainstList, + normalizeDomain, processConfigs, // processConfigs, processDomainList, @@ -754,3 +757,34 @@ describe('generateParentDomains', () => { expect(generateParentDomains(filteredSourceParts)).toStrictEqual(expected); }); }); + +describe('normalizeDomain', () => { + it('should remove trailing dot from domain', () => { + const domain = 'example.com.'; + const result = normalizeDomain(domain); + expect(result).toBe('example.com'); + }); + + it('should return domain unchanged if there is no trailing dot', () => { + const domain = 'example.com'; + const result = normalizeDomain(domain); + expect(result).toBe(domain); + }); +}); + +describe('fetchWithTimeout', () => { + const url = 'https://example.com'; + const timeout = 100; + + afterEach(() => { + nock.cleanAll(); + }); + + it('should fetch successfully if response is within timeout', async () => { + nock(url).get('/').reply(200, 'Success'); + + const result = await fetchWithTimeout(url, timeout); + const text = await result.text(); + expect(text).toBe('Success'); + }); +}); diff --git a/packages/phishing-controller/src/utils.ts b/packages/phishing-controller/src/utils.ts index cebef8cec4..6e001e09d4 100644 --- a/packages/phishing-controller/src/utils.ts +++ b/packages/phishing-controller/src/utils.ts @@ -336,3 +336,38 @@ export const generateParentDomains = ( return domains; }; + +/** + * Normalizes the domain by removing any trailing dot. + * + * @param domain - The domain to normalize. + * @returns The normalized domain. + */ +export const normalizeDomain = (domain: string): string => { + return domain.endsWith('.') ? domain.slice(0, -1) : domain; +}; + +/** + * Fetch with a timeout. + * + * @param url - The URL to fetch. + * @param timeout - The timeout in milliseconds. + * @returns A promise that resolves to the fetch Response. + */ +export async function fetchWithTimeout( + url: string, + timeout: number, +): Promise { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeout); + + try { + const response = await fetch(url, { + signal: controller.signal, + cache: 'no-cache', + }); + return response; + } finally { + clearTimeout(timeoutId); + } +}