From 41c547015df81cf003c2caa8532582642b6d0a4d Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:21:50 -0700 Subject: [PATCH] [PM-32843] Fix phishing blocker not blocking HTTPS variants of HTTP URLs (#19265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(phishing): check alternate protocol when matching URLs against block list The phishing blocker's IndexedDB lookup only checked the exact URL href. When the block list contains http:// entries but the browser auto-redirects to https://, the lookup failed silently. This adds O(1) alternate-protocol lookups (swapping http:// ↔ https://) with the same trailing-slash normalization. [PM-32843] * test(phishing): add unit tests for alternate-protocol URL matching Covers HTTP→HTTPS and HTTPS→HTTP matching, trailing slash normalization with protocol swap, short-circuit when exact match is found, no false positives, and non-HTTP protocol handling. [PM-32843] --- .../services/phishing-data.service.spec.ts | 68 +++++++++++++++++++ .../services/phishing-data.service.ts | 31 +++++++++ 2 files changed, 99 insertions(+) diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts index 0cbb765ce0e..124aa9972e7 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts @@ -254,6 +254,74 @@ describe("PhishingDataService", () => { (PhishingDataService as any).USE_CUSTOM_MATCHER = originalValue; } }); + + it("should detect HTTPS URL when block list contains HTTP version", async () => { + mockIndexedDbService.hasUrl.mockImplementation(async (url: string) => { + return url === "http://phish.com/malicious-page"; + }); + + const url = new URL("https://phish.com/malicious-page"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("https://phish.com/malicious-page"); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/malicious-page"); + }); + + it("should detect HTTP URL when block list contains HTTPS version", async () => { + mockIndexedDbService.hasUrl.mockImplementation(async (url: string) => { + return url === "https://phish.com/malicious-page"; + }); + + const url = new URL("http://phish.com/malicious-page"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/malicious-page"); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("https://phish.com/malicious-page"); + }); + + it("should detect HTTPS URL with trailing slash when block list contains HTTP without trailing slash", async () => { + mockIndexedDbService.hasUrl.mockImplementation(async (url: string) => { + return url === "http://phish.com"; + }); + + // Browsers add trailing slash to root URLs: https://phish.com/ + const url = new URL("https://phish.com"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + }); + + it("should not perform alternate-protocol lookup when exact match is found", async () => { + mockIndexedDbService.hasUrl.mockImplementation(async (url: string) => { + return url === "https://phish.com/exact"; + }); + + const url = new URL("https://phish.com/exact"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + // Should have been called only once (exact match found on first try) + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledTimes(1); + }); + + it("should return false when neither protocol matches", async () => { + mockIndexedDbService.hasUrl.mockResolvedValue(false); + + const url = new URL("https://safe-site.com/page"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(false); + }); + + it("should skip non-http protocols without alternate lookup", async () => { + const url = new URL("ftp://phish.com/file"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(false); + expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled(); + }); }); describe("data updates", () => { diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts index 72415dbbdbe..1014f4cc1fc 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts @@ -182,6 +182,23 @@ export class PhishingDataService { hasUrl = await this.indexedDbService.hasUrl(urlWithoutTrailingSlash); } + // Check alternate protocol: block list may have http:// but browser navigated to https:// (or vice versa) + if (!hasUrl) { + const alternateHref = this.swapProtocol(urlHref); + if (alternateHref) { + hasUrl = await this.indexedDbService.hasUrl(alternateHref); + + if (!hasUrl) { + const alternateWithoutTrailingSlash = alternateHref.endsWith("/") + ? alternateHref.slice(0, -1) + : null; + if (alternateWithoutTrailingSlash) { + hasUrl = await this.indexedDbService.hasUrl(alternateWithoutTrailingSlash); + } + } + } + } + if (hasUrl) { this.logService.info("[PhishingDataService] Found phishing URL: " + urlHref); return true; @@ -210,6 +227,20 @@ export class PhishingDataService { return false; } + /** + * Swaps the protocol of a URL string between http:// and https://. + * Returns null if the URL doesn't start with either protocol. + */ + private swapProtocol(url: string): string | null { + if (url.startsWith("https://")) { + return "http://" + url.slice(8); + } + if (url.startsWith("http://")) { + return "https://" + url.slice(7); + } + return null; + } + // [FIXME] Pull fetches into api service private async fetchPhishingChecksum(type: PhishingResourceType = PhishingResourceType.Domains) { const checksumUrl = getPhishingResources(type)!.checksumUrl;