From e427de5a5fdc3cc6d66abab95e59498755924b04 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Thu, 12 Dec 2024 00:42:55 -0330 Subject: [PATCH] Revert "feat: add websocket support for c2 detection (#28782)" This reverts commit e0f6575a6dc80913532f33202b1d3e91b31137b4. --- app/manifest/v2/_base.json | 2 - app/manifest/v3/_base.json | 4 +- app/scripts/background.js | 2 +- privacy-snapshot.json | 3 +- test/e2e/helpers.js | 44 ----------- test/e2e/tests/phishing-controller/mocks.js | 19 ++--- .../phishing-detection.spec.js | 76 +------------------ 7 files changed, 14 insertions(+), 136 deletions(-) diff --git a/app/manifest/v2/_base.json b/app/manifest/v2/_base.json index 2f41a7e987fa..f29b7458a9e5 100644 --- a/app/manifest/v2/_base.json +++ b/app/manifest/v2/_base.json @@ -66,8 +66,6 @@ "clipboardWrite", "http://*/*", "https://*/*", - "ws://*/*", - "wss://*/*", "activeTab", "webRequest", "webRequestBlocking", diff --git a/app/manifest/v3/_base.json b/app/manifest/v3/_base.json index 89758033f33a..4d6ee38437d3 100644 --- a/app/manifest/v3/_base.json +++ b/app/manifest/v3/_base.json @@ -50,9 +50,7 @@ "http://localhost:8545/", "file://*/*", "http://*/*", - "https://*/*", - "ws://*/*", - "wss://*/*" + "https://*/*" ], "icons": { "16": "images/icon-16.png", diff --git a/app/scripts/background.js b/app/scripts/background.js index 3571be9022fa..e9aaf2cab20b 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -323,7 +323,7 @@ function maybeDetectPhishing(theController) { return {}; }, { - urls: ['http://*/*', 'https://*/*', 'ws://*/*', 'wss://*/*'], + urls: ['http://*/*', 'https://*/*'], }, isManifestV2 ? ['blocking'] : [], ); diff --git a/privacy-snapshot.json b/privacy-snapshot.json index 230634421d52..49eedf275364 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -72,6 +72,5 @@ "unresponsive-rpc.test", "unresponsive-rpc.url", "user-storage.api.cx.metamask.io", - "www.4byte.directory", - "verify.walletconnect.com" + "www.4byte.directory" ] diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 4ade3f2e48ba..b06c29b17acf 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -4,7 +4,6 @@ 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'); @@ -641,48 +640,6 @@ 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}:8081`, - error, - ); - throw error; - } -} - const logInWithBalanceValidation = async (driver, ganacheServer) => { await unlockWallet(driver); // Wait for balance to load @@ -1018,5 +975,4 @@ 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 3165847740bf..fe11118c6fd2 100644 --- a/test/e2e/tests/phishing-controller/mocks.js +++ b/test/e2e/tests/phishing-controller/mocks.js @@ -10,9 +10,7 @@ const { const lastUpdated = 1; const defaultHotlist = { data: [] }; const defaultC2DomainBlocklist = { - recentlyAdded: [ - '33c8e026e76cea2df82322428554c932961cd80080fa379454350d7f13371f36', // hash for malicious.localhost - ], + recentlyAdded: [], recentlyRemoved: [], lastFetchedAt: '2024-08-27T15:30:45Z', }; @@ -97,12 +95,15 @@ async function setupPhishingDetectionMocks( }; }); - await mockServer.forGet(C2_DOMAIN_BLOCKLIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: defaultC2DomainBlocklist, - }; - }); + await mockServer + .forGet(C2_DOMAIN_BLOCKLIST_URL) + .withQuery({ timestamp: '2024-08-27T15:30:45Z' }) + .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 98184b85224e..ad199cea1e70 100644 --- a/test/e2e/tests/phishing-controller/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -2,13 +2,13 @@ const { strict: assert } = require('assert'); const { createServer } = require('node:http'); const { createDeferredPromise } = require('@metamask/utils'); const { until } = require('selenium-webdriver'); + const { defaultGanacheOptions, withFixtures, openDapp, unlockWallet, WINDOW_TITLES, - createWebSocketConnection, } = require('../../helpers'); const FixtureBuilder = require('../../fixture-builder'); const { @@ -315,80 +315,6 @@ describe('Phishing Detection', function () { ); }); - 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', - }); - - const currentUrl = await driver.getCurrentUrl(); - const expectedPortfolioUrl = `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`; - - assert.equal(currentUrl, expectedPortfolioUrl); - }, - ); - }); - - 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(); - - assert.equal(currentUrl, testPageURL); - }, - ); - }); - describe('Phishing redirect protections', function () { /** * Status codes 305 (via Location header) and 306 (Set-Proxy) header do not