From 2d9d6dbba4d009a0ab3f4351fd70ba0254392839 Mon Sep 17 00:00:00 2001 From: AugmentedMode <31675118+AugmentedMode@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:03:17 -0500 Subject: [PATCH 1/7] feat: add websocket support for c2 detection (#28782) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3788 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/")` No support for detecting WebSocket phishing connections. --- WebSocket phishing connections are detected and blocked during the handshake phase. - [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. - [ ] 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. From 4fe3d0ee741ae8757d1630b10338f0a4c45a3f48 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Thu, 12 Dec 2024 09:07:00 -0500 Subject: [PATCH 2/7] 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 From 02ce67a84a9020464254073aa4956b5443cf6896 Mon Sep 17 00:00:00 2001 From: AugmentedMode <31675118+AugmentedMode@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:03:17 -0500 Subject: [PATCH 3/7] feat: add websocket support for c2 detection (#28782) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **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. --- 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, 136 insertions(+), 14 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 e9aaf2cab20b..3571be9022fa 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -323,7 +323,7 @@ function maybeDetectPhishing(theController) { return {}; }, { - urls: ['http://*/*', 'https://*/*'], + urls: ['http://*/*', 'https://*/*', 'ws://*/*', 'wss://*/*'], }, isManifestV2 ? ['blocking'] : [], ); diff --git a/privacy-snapshot.json b/privacy-snapshot.json index 49eedf275364..230634421d52 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -72,5 +72,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..4ade3f2e48ba 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,48 @@ 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 @@ -975,4 +1018,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..98184b85224e 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,6 +315,80 @@ 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 From fe3f93ef674d720c28787ca8fdb169d61b2d367c Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Thu, 12 Dec 2024 12:16:44 -0500 Subject: [PATCH 4/7] fix: redirect of phishing page on websockets --- app/scripts/background.js | 12 +++--------- .../phishing-controller/phishing-detection.spec.js | 14 ++++++-------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 3571be9022fa..550d9e844a4a 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -308,15 +308,9 @@ 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 }; - } - // redirect the whole tab - return { redirectUrl: redirectHref }; + // redirect the whole tab (even if it's a sub_frame request) + redirectTab(details.tabId, redirectHref); + return { cancel: true }; } // redirect the whole tab (even if it's a sub_frame request) redirectTab(details.tabId, redirectHref); diff --git a/test/e2e/tests/phishing-controller/phishing-detection.spec.js b/test/e2e/tests/phishing-controller/phishing-detection.spec.js index 98184b85224e..3a912fbc3d5f 100644 --- a/test/e2e/tests/phishing-controller/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -307,10 +307,9 @@ describe('Phishing Detection', function () { text: 'Back to safety', }); - const currentUrl = await driver.getCurrentUrl(); - const expectedPortfolioUrl = `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`; - - assert.equal(currentUrl, expectedPortfolioUrl); + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); }, ); }); @@ -350,10 +349,9 @@ describe('Phishing Detection', function () { text: 'Back to safety', }); - const currentUrl = await driver.getCurrentUrl(); - const expectedPortfolioUrl = `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`; - - assert.equal(currentUrl, expectedPortfolioUrl); + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); }, ); }); From 9562bac8f519f81848ac6f7530b77424a51b62d0 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Thu, 12 Dec 2024 14:49:16 -0500 Subject: [PATCH 5/7] fix: pr comment for redirects for main frame requests --- app/scripts/background.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 7610b45a3c43..3adfa83fc32c 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -308,7 +308,9 @@ function maybeDetectPhishing(theController) { // blocking is better than tab redirection, as blocking will prevent // the browser from loading the page at all if (isManifestV2) { - // redirect the whole tab (even if it's a sub_frame request) + if (details.type === 'main_frame') { + return { redirectUrl: redirectHref }; + } redirectTab(details.tabId, redirectHref); return { cancel: true }; } From 4df7c11910ff2484f21a5dea9f65be65d12dee36 Mon Sep 17 00:00:00 2001 From: AugmentedMode <31675118+AugmentedMode@users.noreply.github.com> Date: Thu, 12 Dec 2024 15:26:55 -0500 Subject: [PATCH 6/7] chore: add context for redirects Co-authored-by: Mark Stacey --- app/scripts/background.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/scripts/background.js b/app/scripts/background.js index 3adfa83fc32c..c6a3bccd46aa 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -308,6 +308,9 @@ function maybeDetectPhishing(theController) { // blocking is better than tab redirection, as blocking will prevent // the browser from loading the page at all if (isManifestV2) { + // 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 }; } From 35464871dc464e9b728b2e5a4a0000d046b74640 Mon Sep 17 00:00:00 2001 From: augmentedmode Date: Thu, 12 Dec 2024 15:40:56 -0500 Subject: [PATCH 7/7] fix: wrong port in console log --- test/e2e/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 5fa0b7969504..0a915c0fba29 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -671,7 +671,7 @@ async function createWebSocketConnection(driver, hostname) { }, hostname); } catch (error) { console.error( - `Failed to execute WebSocket connection script for ws://${hostname}:8081`, + `Failed to execute WebSocket connection script for ws://${hostname}:8000`, error, ); throw error;