From 37e51c10c78424ab7d1bec80078099278ead40f2 Mon Sep 17 00:00:00 2001 From: AugmentedMode <31675118+AugmentedMode@users.noreply.github.com> Date: Fri, 13 Dec 2024 05:40:50 -0500 Subject: [PATCH] fix: [cherry-pick] add websocket support for c2 detection (#28782) (#29173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry-picks #28782 ## **Description** This pull request adds WebSocket support to the MetaMask extension's phishing detection functionality. Scammers have started using WebSocket connections for command-and-control (C2) operations to bypass traditional HTTP-based phishing detection. This PR allows the extension to intercept and block WebSocket handshake requests (`ws://` and `wss://`) in addition to HTTP/HTTPS requests. The key changes include: 1. Adding WebSocket schemes (`ws://*/*` and `wss://*/*`) to the `urls` filter in `background.js`. 2. Updating the `manifest.json` to include WebSocket permissions in the `host_permissions` field. This ensures that malicious WebSocket connections can be detected and blocked. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28782?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3788 ## **Manual testing steps** 1. Navigate to `example.com` 2. Initiate a WebSocket connection to a known safe domain (e.g., `wss://example.com`) and verify it works as expected by going to the `console` via right clicking and hitting inspect. Then type into the console `new WebSocket("https://example.com/")` 3. Attempt a WebSocket connection to a domain flagged as phishing, and verify the connection is blocked and appropriate warnings are displayed by going to the `console` via right clicking and hitting inspect. Then type into the console `new WebSocket("https://walietconnectapi.com/")` ## **Screenshots/Recordings** ### **Before** No support for detecting WebSocket phishing connections. --- ### **After** WebSocket phishing connections are detected and blocked during the handshake phase. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: Mark Stacey --- app/manifest/v2/_base.json | 2 + app/manifest/v3/_base.json | 4 +- app/scripts/background.js | 17 ++-- privacy-snapshot.json | 3 +- test/e2e/helpers.js | 39 ++++++++++ test/e2e/tests/phishing-controller/mocks.js | 19 +++-- .../phishing-detection.spec.js | 77 ++++++++++++++++++- 7 files changed, 138 insertions(+), 23 deletions(-) diff --git a/app/manifest/v2/_base.json b/app/manifest/v2/_base.json index f29b7458a9e5..2f41a7e987fa 100644 --- a/app/manifest/v2/_base.json +++ b/app/manifest/v2/_base.json @@ -66,6 +66,8 @@ "clipboardWrite", "http://*/*", "https://*/*", + "ws://*/*", + "wss://*/*", "activeTab", "webRequest", "webRequestBlocking", diff --git a/app/manifest/v3/_base.json b/app/manifest/v3/_base.json index 4d6ee38437d3..89758033f33a 100644 --- a/app/manifest/v3/_base.json +++ b/app/manifest/v3/_base.json @@ -50,7 +50,9 @@ "http://localhost:8545/", "file://*/*", "http://*/*", - "https://*/*" + "https://*/*", + "ws://*/*", + "wss://*/*" ], "icons": { "16": "images/icon-16.png", diff --git a/app/scripts/background.js b/app/scripts/background.js index 2b2f5c4693df..938cac879f25 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -315,22 +315,21 @@ function maybeDetectPhishing(theController) { // blocking is better than tab redirection, as blocking will prevent // the browser from loading the page at all if (isManifestV2) { - if (details.type === 'sub_frame') { - // redirect the entire tab to the - // phishing warning page instead. - redirectTab(details.tabId, redirectHref); - // don't let the sub_frame load at all - return { cancel: true }; + // We can redirect `main_frame` requests directly to the warning page. + // For non-`main_frame` requests (e.g. `sub_frame` or WebSocket), we cancel them + // and redirect the whole tab asynchronously so that the user sees the warning. + if (details.type === 'main_frame') { + return { redirectUrl: redirectHref }; } - // redirect the whole tab - return { redirectUrl: redirectHref }; + redirectTab(details.tabId, redirectHref); + return { cancel: true }; } // redirect the whole tab (even if it's a sub_frame request) redirectTab(details.tabId, redirectHref); return {}; }, { - urls: ['http://*/*', 'https://*/*'], + urls: ['http://*/*', 'https://*/*', 'ws://*/*', 'wss://*/*'], }, isManifestV2 ? ['blocking'] : [], ); diff --git a/privacy-snapshot.json b/privacy-snapshot.json index 6ee430ca943c..8ae10de304df 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -71,5 +71,6 @@ "unresponsive-rpc.test", "unresponsive-rpc.url", "user-storage.api.cx.metamask.io", - "www.4byte.directory" + "www.4byte.directory", + "verify.walletconnect.com" ] diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index b06c29b17acf..6900ac243629 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -4,6 +4,7 @@ const BigNumber = require('bignumber.js'); const mockttp = require('mockttp'); const detectPort = require('detect-port'); const { difference } = require('lodash'); +const WebSocket = require('ws'); const createStaticServer = require('../../development/create-static-server'); const { setupMocking } = require('./mock-e2e'); const { Ganache } = require('./seeder/ganache'); @@ -640,6 +641,43 @@ async function unlockWallet( } } +/** + * Simulates a WebSocket connection by executing a script in the browser context. + * + * @param {WebDriver} driver - The WebDriver instance. + * @param {string} hostname - The hostname to connect to. + */ +async function createWebSocketConnection(driver, hostname) { + try { + await driver.executeScript(async (wsHostname) => { + const url = `ws://${wsHostname}:8000`; + const socket = new WebSocket(url); + socket.onopen = () => { + console.log('WebSocket connection opened'); + socket.send('Hello, server!'); + }; + socket.onerror = (error) => { + console.error( + 'WebSocket error:', + error.message || 'Connection blocked', + ); + }; + socket.onmessage = (event) => { + console.log('Message received from server:', event.data); + }; + socket.onclose = () => { + console.log('WebSocket connection closed'); + }; + }, hostname); + } catch (error) { + console.error( + `Failed to execute WebSocket connection script for ws://${hostname}:8000`, + error, + ); + throw error; + } +} + const logInWithBalanceValidation = async (driver, ganacheServer) => { await unlockWallet(driver); // Wait for balance to load @@ -975,4 +1013,5 @@ module.exports = { tempToggleSettingRedesignedTransactionConfirmations, openMenuSafe, sentryRegEx, + createWebSocketConnection, }; diff --git a/test/e2e/tests/phishing-controller/mocks.js b/test/e2e/tests/phishing-controller/mocks.js index fe11118c6fd2..3165847740bf 100644 --- a/test/e2e/tests/phishing-controller/mocks.js +++ b/test/e2e/tests/phishing-controller/mocks.js @@ -10,7 +10,9 @@ const { const lastUpdated = 1; const defaultHotlist = { data: [] }; const defaultC2DomainBlocklist = { - recentlyAdded: [], + recentlyAdded: [ + '33c8e026e76cea2df82322428554c932961cd80080fa379454350d7f13371f36', // hash for malicious.localhost + ], recentlyRemoved: [], lastFetchedAt: '2024-08-27T15:30:45Z', }; @@ -95,15 +97,12 @@ async function setupPhishingDetectionMocks( }; }); - await mockServer - .forGet(C2_DOMAIN_BLOCKLIST_URL) - .withQuery({ timestamp: '2024-08-27T15:30:45Z' }) - .thenCallback(() => { - return { - statusCode: 200, - json: defaultC2DomainBlocklist, - }; - }); + await mockServer.forGet(C2_DOMAIN_BLOCKLIST_URL).thenCallback(() => { + return { + statusCode: 200, + json: defaultC2DomainBlocklist, + }; + }); await mockServer .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') diff --git a/test/e2e/tests/phishing-controller/phishing-detection.spec.js b/test/e2e/tests/phishing-controller/phishing-detection.spec.js index ad199cea1e70..1fabd8f901bc 100644 --- a/test/e2e/tests/phishing-controller/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -9,6 +9,7 @@ const { openDapp, unlockWallet, WINDOW_TITLES, + createWebSocketConnection, } = require('../../helpers'); const FixtureBuilder = require('../../fixture-builder'); const { @@ -307,10 +308,82 @@ describe('Phishing Detection', function () { text: 'Back to safety', }); + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); + }, + ); + }); + + it('should block a website that makes a websocket connection to a malicious command and control server', async function () { + const testPageURL = 'http://localhost:8080'; + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.fullTitle(), + testSpecificMock: async (mockServer) => { + await mockServer.forAnyWebSocket().thenEcho(); + await setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + }); + }, + dapp: true, + }, + async ({ driver }) => { + await unlockWallet(driver); + + await driver.openNewPage(testPageURL); + + await createWebSocketConnection(driver, 'malicious.localhost'); + + await driver.switchToWindowWithTitle( + 'MetaMask Phishing Detection', + 10000, + ); + + await driver.waitForSelector({ + testId: 'unsafe-continue-loaded', + }); + + await driver.clickElement({ + text: 'Back to safety', + }); + + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); + }, + ); + }); + + it('should not block a website that makes a safe WebSocket connection', async function () { + const testPageURL = 'http://localhost:8080/'; + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.fullTitle(), + testSpecificMock: async (mockServer) => { + await mockServer.forAnyWebSocket().thenEcho(); + await setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + }); + }, + dapp: true, + }, + async ({ driver }) => { + await unlockWallet(driver); + + await driver.openNewPage(testPageURL); + + await createWebSocketConnection(driver, 'safe.localhost'); + + await driver.wait(until.titleIs(WINDOW_TITLES.TestDApp), 10000); + const currentUrl = await driver.getCurrentUrl(); - const expectedPortfolioUrl = `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`; - assert.equal(currentUrl, expectedPortfolioUrl); + assert.equal(currentUrl, testPageURL); }, ); });