Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement client side malicious network request detection #25839

Merged
merged 36 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
56d4283
wip: mvp to show blocking c2 domains
Jul 12, 2024
70467d7
wip: mvp to show blocking c2 domains
Jul 12, 2024
f2396ef
wip: all logic is not abstracted within the phishing controller withi…
Jul 15, 2024
7178989
feat: succesfully redirects users to the phishing page if the domain …
Jul 15, 2024
3beabd1
feat: added a reason to meta metrics for when we block a website
Jul 15, 2024
25680ea
wip
AugmentedMode Jul 29, 2024
24075b9
wip
AugmentedMode Aug 15, 2024
0f3f5ae
chore: merge main into feature branch
AugmentedMode Aug 28, 2024
4eeac70
chore: fix test that seems to be already broken for now
AugmentedMode Aug 28, 2024
c835b62
wip: send url instead of hostname to check phishing controller
AugmentedMode Aug 28, 2024
ac4534d
chore: remove local phishing controller and pull in phishing controll…
AugmentedMode Aug 28, 2024
4e78351
chore: fix merge conflicts
AugmentedMode Aug 28, 2024
777fbb0
chore: support new test function where we have to send the url instea…
AugmentedMode Aug 28, 2024
abd8869
Update LavaMoat policies
metamaskbot Aug 28, 2024
7e8894d
chore: Update privacy-snapshot.json with new API endpoint for client-…
AugmentedMode Aug 28, 2024
20d1b40
Merge branch 'feat/client-side-detection' of https://github.com/MetaM…
AugmentedMode Aug 28, 2024
c41895d
chore: fix failing tests
AugmentedMode Aug 28, 2024
7465082
chore: add tests to cover c2 domian detection
AugmentedMode Aug 28, 2024
10da980
chore: yarn dedupe
AugmentedMode Aug 28, 2024
f5ed2ab
Update LavaMoat policies
metamaskbot Aug 28, 2024
ddda6f7
chore: fix privacy snapshot url for client-side-detection.api.cx.meta…
AugmentedMode Aug 28, 2024
bab80a3
chore: remove all references to phishfort
AugmentedMode Aug 28, 2024
dc7f4b5
fix: mock timestamp query
AugmentedMode Aug 29, 2024
6fbc046
chore: bring back correct phishing controller version
AugmentedMode Aug 29, 2024
bb4b2f4
chore: update phishing controller version to 12.0.1
AugmentedMode Aug 29, 2024
a0a27ed
chore: add tests for ipfs blocking
AugmentedMode Aug 29, 2024
18f0ca2
Merge branch 'develop' into feat/client-side-detection
AugmentedMode Aug 29, 2024
3d4bcd9
chore: add tests for ipfs blocking
AugmentedMode Aug 29, 2024
08c9f3d
Merge branch 'feat/client-side-detection' of https://github.com/MetaM…
AugmentedMode Aug 29, 2024
b47587e
chore: fix ipfs tests
AugmentedMode Aug 29, 2024
0b5e4ee
chore: add more ipfs tests
AugmentedMode Aug 29, 2024
fc58e1f
chore: fix pr comments
AugmentedMode Aug 29, 2024
0f0794a
Merge branch 'develop' into feat/client-side-detection
AugmentedMode Aug 29, 2024
f5fde7b
chore: fix pr comments
AugmentedMode Aug 29, 2024
1ec468c
Merge branch 'feat/client-side-detection' of https://github.com/MetaM…
AugmentedMode Aug 29, 2024
cebf9f0
Merge branch 'develop' into feat/client-side-detection
AugmentedMode Aug 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,35 @@ function maybeDetectPhishing(theController) {
}

theController.phishingController.maybeUpdateState();
const phishingTestResponse =
theController.phishingController.test(hostname);
if (!phishingTestResponse?.result) {
const phishingTestResponse = theController.phishingController.test(
details.url,
);

const blockedRequestResponse =
theController.phishingController.isBlockedRequest(details.url);

// if the request is not blocked, and the phishing test is not blocked, return and don't show the phishing screen
if (!phishingTestResponse?.result && !blockedRequestResponse.result) {
return {};
}

// Determine the block reason based on the type
let blockReason;
if (phishingTestResponse?.result && blockedRequestResponse.result) {
blockReason = `${phishingTestResponse.type} and ${blockedRequestResponse.type}`;
Mrtenz marked this conversation as resolved.
Show resolved Hide resolved
} else if (phishingTestResponse?.result) {
blockReason = phishingTestResponse.type;
} else {
blockReason = blockedRequestResponse.type;
}

theController.metaMetricsController.trackEvent({
// should we differentiate between background redirection and content script redirection?
event: MetaMetricsEventName.PhishingPageDisplayed,
category: MetaMetricsEventCategory.Phishing,
properties: {
url: hostname,
reason: blockReason,
},
});
const querystring = new URLSearchParams({ hostname, href });
Expand All @@ -297,7 +314,10 @@ function maybeDetectPhishing(theController) {
redirectTab(details.tabId, redirectHref);
return {};
},
{ types: ['main_frame', 'sub_frame'], urls: ['http://*/*', 'https://*/*'] },
{
types: ['main_frame', 'sub_frame', 'xmlhttprequest'],
urls: ['http://*/*', 'https://*/*'],
},
isManifestV2 ? ['blocking'] : [],
);
}
Expand Down
42 changes: 40 additions & 2 deletions app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ListNames, PhishingController } from '@metamask/phishing-controller';
import { ControllerMessenger } from '@metamask/base-controller';
import { isBlockedUrl } from './isBlockedUrl';
import { isBlockedUrl, isC2DomainBlocked } from './isBlockedUrl';

describe('isBlockedUrl', () => {
const messenger = new ControllerMessenger();
Expand All @@ -10,17 +10,26 @@ describe('isBlockedUrl', () => {
allowedEvents: [],
});
const phishingController = new PhishingController({
// @ts-expect-error TODO: Resolve/patch mismatch between messenger types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This is caused by the major version difference in the base-controller versions used in phishing-controller (v6) vs. extension (v5).

The only reason we're not seeing the same error in metamask-controller.js is because the file hasn't been converted to TypeScript.

messenger: phishingControllerMessenger,
state: {
phishingLists: [
{
blocklist: ['https://metamask.test'],
blocklist: [
'metamask.test',
'QmYwAPJzv5CZsnAzt8auVTL6aKqgfZY5vHBYdbyz4ySxTm',
'ipfs://QmXbVAkGZMz6p8nJ3wXBng4JwvBqZWkFwnDMevL7Tz5w8y',
'QmT78zSuBmuS4z925WJg3vNLRiT4Mj6apb5iS4iykKs4n8',
],
allowlist: [],
fuzzylist: [],
tolerance: 0,
version: 1,
lastUpdated: 0,
name: ListNames.MetaMask,
c2DomainBlocklist: [
'a379a6f6eeafb9a55e378c118034e2751e682fab9f2d30ab13d2125586ce1947',
],
},
],
},
Expand All @@ -32,6 +41,16 @@ describe('isBlockedUrl', () => {
['https://metamask.io', false],
['https://metamask.test', true],
['sftp://metamask.io', true],
['ipfs://QmYwAPJzv5CZsnAzt8auVTL6aKqgfZY5vHBYdbyz4ySxTm', true],
['ipfs://QmXbVAkGZMz6p8nJ3wXBng4JwvBqZWkFgnDMevL7Tz5w8y', true],
[
'https://ipfs.io/ipfs/QmT78zSuBmuS4z925WJg3vNLRiT4Mj6apb5iS4iykKs4n8',
true,
],
[
'https://ipfs.io/ipfs/QmT78zSuBmuS4zdsf925WJsadfsdfg3vNLRiT4Mj6apb5iS4iykKs4n8',
false,
],
['', true],
['1', true],
[undefined, true],
Expand All @@ -53,4 +72,23 @@ describe('isBlockedUrl', () => {
);
expect(result).toEqual(expected);
});

// @ts-expect-error This is missing from the Mocha type definitions
it.each([
['https://example.com', true],
['https://metamask.io', false],
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
])('"%s" is blocked: %s', async (url: any, expected: boolean) => {
const result = await isC2DomainBlocked(
url,
async () => {
return await phishingController.maybeUpdateState();
},
(origin: string) => {
return phishingController.isBlockedRequest(origin);
},
);
expect(result).toEqual(expected);
});
});
28 changes: 28 additions & 0 deletions app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,31 @@ export const isBlockedUrl = async (
return false;
}
};

/**
* Checks weather a given URL is blocked due to making a network request to a
* known C2 domain.
*
* @param url - The URL to check.
* @param maybeUpdateState - A function that updates the phishing controller state.
* @param testC2Domain - A function that tests if a URL is a known malicious C2 domain.
* @returns Returns a promise which resolves to `true` if the URL is blocked
* due to making a network request to a known malicious C2 domain. Otherwise,
* resolves to `false`.
*/
export const isC2DomainBlocked = async (
url: string,
maybeUpdateState: () => ReturnType<PhishingController['maybeUpdateState']>,
testC2Domain: (
url: string,
) => ReturnType<PhishingController['isBlockedRequest']>,
): Promise<boolean> => {
try {
// check if the url is in the phishing list
await maybeUpdateState();
return testC2Domain(url).result;
} catch (error) {
console.error('Invalid URL passed into snap-keyring:', error);
return false;
}
};
4 changes: 0 additions & 4 deletions app/scripts/metamask-controller.actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,6 @@ describe('MetaMaskController', function () {
blocklist: ['127.0.0.1'],
name: ListNames.MetaMask,
},
phishfort_hotlist: {
blocklist: [],
name: ListNames.Phishfort,
},
}),
)
.get(METAMASK_HOTLIST_DIFF_FILE)
Expand Down
6 changes: 3 additions & 3 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2682,7 +2682,7 @@ export default class MetamaskController extends EventEmitter {
'PhishingController:maybeUpdateState',
);
},
isOnPhishingList: (origin) => {
isOnPhishingList: (sender) => {
const { usePhishDetect } =
this.preferencesController.store.getState();

Expand All @@ -2692,7 +2692,7 @@ export default class MetamaskController extends EventEmitter {

return this.controllerMessenger.call(
'PhishingController:testOrigin',
origin,
sender.url,
).result;
},
createInterface: this.controllerMessenger.call.bind(
Expand Down Expand Up @@ -4999,7 +4999,7 @@ export default class MetamaskController extends EventEmitter {
const { hostname } = new URL(sender.url);
this.phishingController.maybeUpdateState();
// Check if new connection is blocked if phishing detection is on
const phishingTestResponse = this.phishingController.test(hostname);
const phishingTestResponse = this.phishingController.test(sender.url);
if (phishingTestResponse?.result) {
this.sendPhishingWarning(connectionStream, hostname);
this.metaMetricsController.trackEvent({
Expand Down
4 changes: 0 additions & 4 deletions app/scripts/metamask-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,6 @@ describe('MetaMaskController', () => {
blocklist: ['test.metamask-phishing.io'],
name: ListNames.MetaMask,
},
phishfort_hotlist: {
blocklist: [],
name: ListNames.Phishfort,
},
}),
)
.get(METAMASK_HOTLIST_DIFF_FILE)
Expand Down
39 changes: 9 additions & 30 deletions lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2140,36 +2140,25 @@
},
"@metamask/phishing-controller": {
"globals": {
"TextEncoder": true,
"URL": true,
"fetch": true
},
"packages": {
"@metamask/base-controller": true,
"@metamask/phishing-controller>@metamask/controller-utils": true,
"@metamask/phishing-warning>eth-phishing-detect": true,
"@ethereumjs/tx>ethereum-cryptography": true,
"@metamask/controller-utils": true,
"@metamask/phishing-controller>@metamask/base-controller": true,
"@metamask/phishing-controller>fastest-levenshtein": true,
"@noble/hashes": true,
"punycode": true
}
},
"@metamask/phishing-controller>@metamask/controller-utils": {
"@metamask/phishing-controller>@metamask/base-controller": {
"globals": {
"URL": true,
"console.error": true,
"fetch": true,
"setTimeout": true
},
"packages": {
"@ethereumjs/tx>@ethereumjs/util": true,
"@metamask/controller-utils>@spruceid/siwe-parser": true,
"@metamask/ethjs>@metamask/ethjs-unit": true,
"@metamask/utils": true,
"bn.js": true,
"browserify>buffer": true,
"eslint>fast-deep-equal": true,
"eth-ens-namehash": true
}
},
"@metamask/phishing-warning>eth-phishing-detect": {
"packages": {
"eslint>optionator>fast-levenshtein": true
"immer": true
}
},
"@metamask/post-message-stream": {
Expand Down Expand Up @@ -4292,16 +4281,6 @@
"koa>is-generator-function>has-tostringtag": true
}
},
"eslint>optionator>fast-levenshtein": {
"globals": {
"Intl": true,
"Levenshtein": "write",
"console.log": true,
"define": true,
"importScripts": true,
"postMessage": true
}
},
"eth-ens-namehash": {
"globals": {
"name": "write"
Expand Down
39 changes: 9 additions & 30 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2140,36 +2140,25 @@
},
"@metamask/phishing-controller": {
"globals": {
"TextEncoder": true,
"URL": true,
"fetch": true
},
"packages": {
"@metamask/base-controller": true,
"@metamask/phishing-controller>@metamask/controller-utils": true,
"@metamask/phishing-warning>eth-phishing-detect": true,
"@ethereumjs/tx>ethereum-cryptography": true,
"@metamask/controller-utils": true,
"@metamask/phishing-controller>@metamask/base-controller": true,
"@metamask/phishing-controller>fastest-levenshtein": true,
"@noble/hashes": true,
"punycode": true
}
},
"@metamask/phishing-controller>@metamask/controller-utils": {
"@metamask/phishing-controller>@metamask/base-controller": {
"globals": {
"URL": true,
"console.error": true,
"fetch": true,
"setTimeout": true
},
"packages": {
"@ethereumjs/tx>@ethereumjs/util": true,
"@metamask/controller-utils>@spruceid/siwe-parser": true,
"@metamask/ethjs>@metamask/ethjs-unit": true,
"@metamask/utils": true,
"bn.js": true,
"browserify>buffer": true,
"eslint>fast-deep-equal": true,
"eth-ens-namehash": true
}
},
"@metamask/phishing-warning>eth-phishing-detect": {
"packages": {
"eslint>optionator>fast-levenshtein": true
"immer": true
}
},
"@metamask/post-message-stream": {
Expand Down Expand Up @@ -4292,16 +4281,6 @@
"koa>is-generator-function>has-tostringtag": true
}
},
"eslint>optionator>fast-levenshtein": {
"globals": {
"Intl": true,
"Levenshtein": "write",
"console.log": true,
"define": true,
"importScripts": true,
"postMessage": true
}
},
"eth-ens-namehash": {
"globals": {
"name": "write"
Expand Down
39 changes: 9 additions & 30 deletions lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2140,36 +2140,25 @@
},
"@metamask/phishing-controller": {
"globals": {
"TextEncoder": true,
"URL": true,
"fetch": true
},
"packages": {
"@metamask/base-controller": true,
"@metamask/phishing-controller>@metamask/controller-utils": true,
"@metamask/phishing-warning>eth-phishing-detect": true,
"@ethereumjs/tx>ethereum-cryptography": true,
"@metamask/controller-utils": true,
"@metamask/phishing-controller>@metamask/base-controller": true,
"@metamask/phishing-controller>fastest-levenshtein": true,
"@noble/hashes": true,
"punycode": true
}
},
"@metamask/phishing-controller>@metamask/controller-utils": {
"@metamask/phishing-controller>@metamask/base-controller": {
"globals": {
"URL": true,
"console.error": true,
"fetch": true,
"setTimeout": true
},
"packages": {
"@ethereumjs/tx>@ethereumjs/util": true,
"@metamask/controller-utils>@spruceid/siwe-parser": true,
"@metamask/ethjs>@metamask/ethjs-unit": true,
"@metamask/utils": true,
"bn.js": true,
"browserify>buffer": true,
"eslint>fast-deep-equal": true,
"eth-ens-namehash": true
}
},
"@metamask/phishing-warning>eth-phishing-detect": {
"packages": {
"eslint>optionator>fast-levenshtein": true
"immer": true
}
},
"@metamask/post-message-stream": {
Expand Down Expand Up @@ -4292,16 +4281,6 @@
"koa>is-generator-function>has-tostringtag": true
}
},
"eslint>optionator>fast-levenshtein": {
"globals": {
"Intl": true,
"Levenshtein": "write",
"console.log": true,
"define": true,
"importScripts": true,
"postMessage": true
}
},
"eth-ens-namehash": {
"globals": {
"name": "write"
Expand Down
Loading
Loading