Skip to content

Commit

Permalink
Refactor URL query parameter filtering in phishingService.ts (#154)
Browse files Browse the repository at this point in the history
* Refactor URL query parameter filtering in phishingService.ts

* Refactor URL query parameter filtering and added tests
  • Loading branch information
AugmentedMode authored Apr 12, 2024
1 parent b957639 commit 72c519d
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 11 deletions.
99 changes: 91 additions & 8 deletions src/lib/helpers/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,17 @@ export function urlIsPhishingWarning(url: string): boolean {
return false;
}


const sha256 = async (domain: string) => {
const hash = await crypto.subtle.digest('SHA-256', new
TextEncoder().encode(domain));
return Array.from(new Uint8Array(hash)).map(b =>
b.toString(16).padStart(2, '0')).join('');
}
const hash = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(domain));
return Array.from(new Uint8Array(hash))
.map((b) => b.toString(16).padStart(2, '0'))
.join('');
};

type BlocklistCheckResponse = {
blocked: boolean;
hash: string;
}
};

export const isBlocked = async (urlString: string): Promise<BlocklistCheckResponse> => {
const blocklist = await localStorageHelpers.get<string[]>(WgKeys.RequestsBlocklist);
Expand All @@ -108,6 +107,90 @@ export const isBlocked = async (urlString: string): Promise<BlocklistCheckRespon

return {
blocked,
hash
hash,
};
};

export function filterURLQueryParameters(input: string): string {
const filteredParams = new Set(
[
'username',
'user',
'email',
'fullname',
'name',
'first_name',
'last_name',
'phone',
'phone_number',
'address',
'city',
'state',
'zipcode',
'postal_code',
'country',
'ssn',
'passport',
'driver_license',
'credit_card',
'password',
'token',
'auth',
'authentication',
'session',
'bank_account',
'bvn',
'routing_number',
'transaction_id',
'medical_record',
'health_insurance',
'patient_id',
'national_id',
'tax_id',
'employee_id',
'member_id',
'ip_address',
'mac_address',
'device_id',
'login',
'subscriber_id',
'member',
'profile_id',
'user_id',
'api_key',
'client_id',
'client_secret',
'access_token',
'refresh_token',
'dob',
'gender',
'race',
'nationality',
'marital_status',
'wallet_address',
'public_key',
'tx_id',
'transaction_hash',
'nonce',
'contract_address',
'token_id',
'signature',
'seed_phrase',
'node_id',
'chain_id',
].map((p) => p.toLowerCase())
);

const url = new URL(input);
const params = url.searchParams;

Array.from(params.keys()).forEach((key) => {
if (filteredParams.has(key.toLowerCase())) {
params.delete(key);
}
});

url.search = params.toString();

return url.toString();
}
11 changes: 8 additions & 3 deletions src/services/phishing/phishingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AlertHandler } from '../../lib/helpers/chrome/alertHandler';
import localStorageHelpers from '../../lib/helpers/chrome/localStorage';
import { WgKeys } from '../../lib/helpers/chrome/localStorageKeys';
import { getDomainNameFromURL } from '../../lib/helpers/phishing/parseDomainHelper';
import { urlIsPhishingWarning } from '../../lib/helpers/util';
import { filterURLQueryParameters, urlIsPhishingWarning } from '../../lib/helpers/util';
import { ExtensionSettings } from '../../lib/settings';
import { AlertDetail } from '../../models/Alert';
import { RecommendedAction } from '../../models/PhishingResponse';
Expand All @@ -22,7 +22,11 @@ export async function checkUrlForPhishing(tab: chrome.tabs.Tab) {
// do not scan if domain is browser native page
if (hasBrowserPrefix(url)) return;

const pdsResponse = await domainScan(url);
// Scrub the URL of sensitive query parameters before any scan
const scrubbedUrl = filterURLQueryParameters(url);

const pdsResponse = await domainScan(scrubbedUrl);

chrome.storage.local.set({ currentSite: pdsResponse });

const recentlyCreatedWarning = pdsResponse?.riskFactors?.find(
Expand Down Expand Up @@ -59,7 +63,8 @@ export async function checkUrlForPhishing(tab: chrome.tabs.Tab) {
const currentUrl = (await chrome.tabs.get(tab.id)).url;
const currentDomainName = getDomainNameFromURL(currentUrl || '');
const tabExists = currentDomainName === pdsResponse?.domainName;
const shouldBlock = pdsResponse?.recommendedAction === RecommendedAction.Block && settings?.phishingDetection && tabExists;
const shouldBlock =
pdsResponse?.recommendedAction === RecommendedAction.Block && settings?.phishingDetection && tabExists;
const criticalRiskFactor: RiskFactor | undefined = pdsResponse?.riskFactors?.find(
(warning) => warning.severity === Severity.Critical
);
Expand Down
74 changes: 74 additions & 0 deletions src/tests/phishingService.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { filterURLQueryParameters } from '../lib/helpers/util';

describe('filterURLQueryParameters', () => {
it('should remove sensitive query parameters', () => {
const url = 'https://example.com?username=user&[email protected]&non_sensitive=data';
const expected = 'https://example.com/?non_sensitive=data';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle URLs without query parameters', () => {
const url = 'https://example.com';
const expected = 'https://example.com/';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle URLs with mixed case query parameters', () => {
const url = 'https://example.com?UserName=user&[email protected]';
const expected = 'https://example.com/';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should not remove non-sensitive query parameters', () => {
const url = 'https://example.com?data=value&info=morevalue';
const expected = 'https://example.com/?data=value&info=morevalue';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle multiple sensitive query parameters', () => {
const url = 'https://example.com?username=user&password=pass123&token=abc123';
const expected = 'https://example.com/';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle encoded and special characters in query parameters', () => {
const url = 'https://example.com?username=user&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback';
const expected = 'https://example.com/?redirect_uri=https%3A%2F%2Fexample.com%2Fcallback';
expect(filterURLQueryParameters(url)).toBe(expected);
});
});

it('should return the same URL if there are no sensitive query parameters', () => {
const url = 'https://example.com/?data=value&info=morevalue';
expect(filterURLQueryParameters(url)).toBe(url);
});

it('should handle URLs with empty query parameters correctly', () => {
const url = 'https://example.com?username=&token=';
const expected = 'https://example.com/';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle multiple instances of the same sensitive parameter', () => {
const url = 'https://example.com?user=user1&user=user2&info=morevalue';
const expected = 'https://example.com/?info=morevalue';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should preserve hash fragments in URLs', () => {
const url = 'https://example.com?username=user#contact';
const expected = 'https://example.com/#contact';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should correctly handle URLs with complex encoded components', () => {
const url = 'https://example.com?redirect_uri=https%3A%2F%2Fexample.com%2Fpath%3Farg%3Dvalue&user=user';
const expected = 'https://example.com/?redirect_uri=https%3A%2F%2Fexample.com%2Fpath%3Farg%3Dvalue';
expect(filterURLQueryParameters(url)).toBe(expected);
});

it('should handle complex real-world URLs', () => {
const url = 'https://example.com?user=user&password=pass&token=123&lang=en&session=abc123&page=1';
const expected = 'https://example.com/?lang=en&page=1';
expect(filterURLQueryParameters(url)).toBe(expected);
});

0 comments on commit 72c519d

Please sign in to comment.