mirror of
https://github.com/bitwarden/clients.git
synced 2026-07-07 21:10:55 +08:00
[PM-32843] Fix phishing blocker not blocking HTTPS variants of HTTP URLs (#19265)
* 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]
This commit is contained in:
parent
6428997a00
commit
41c547015d
@ -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", () => {
|
||||
|
||||
@ -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;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user