From c1ca70d7325577835a23c1fae2b0b9b10df54490 Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 2 May 2022 11:23:20 -1000 Subject: [PATCH 01/16] phishing-detect - validate redirect url protocol --- app/scripts/phishing-detect.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/scripts/phishing-detect.js b/app/scripts/phishing-detect.js index 68c30d501..c076a74d4 100644 --- a/app/scripts/phishing-detect.js +++ b/app/scripts/phishing-detect.js @@ -35,6 +35,16 @@ function start() { params: [suspect.hostname], id: createRandomId(), }); - window.location.href = suspect.href; + const redirectTarget = new URL(suspect.href, window.location.href); + // validate redirect url + const invalidProtocol = !(['https:', 'http:'].includes(redirectTarget.protocol)); + // if in valid, show warning and abort + if (invalidProtocol) { + // we intentionally dont display to the user any potential attacker-written content here + console.error(`Invalid redirect url.`); + return; + }; + // use the validated url instance + window.location.href = redirectTarget.href; }); } From 0110bd957133ecf5dec9eb560a6dfe160c851c65 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 3 May 2022 14:05:40 -0230 Subject: [PATCH 02/16] Fix lint errors --- app/scripts/phishing-detect.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/scripts/phishing-detect.js b/app/scripts/phishing-detect.js index c076a74d4..f926b234d 100644 --- a/app/scripts/phishing-detect.js +++ b/app/scripts/phishing-detect.js @@ -37,13 +37,15 @@ function start() { }); const redirectTarget = new URL(suspect.href, window.location.href); // validate redirect url - const invalidProtocol = !(['https:', 'http:'].includes(redirectTarget.protocol)); + const invalidProtocol = !['https:', 'http:'].includes( + redirectTarget.protocol, + ); // if in valid, show warning and abort if (invalidProtocol) { // we intentionally dont display to the user any potential attacker-written content here console.error(`Invalid redirect url.`); return; - }; + } // use the validated url instance window.location.href = redirectTarget.href; }); From 900ac4596b02c755e2231e32d43bcd7b8ae84a20 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 3 May 2022 13:56:25 -0230 Subject: [PATCH 03/16] Version v10.14.1 This is a rollback release to v10.13.0 --- CHANGELOG.md | 7 ++++++- package.json | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0dbe5484..1e9550f1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.14.1] +### Changed +- This version was used to rollback from v10.14.0 to v10.13.0. + ## [10.13.0] ### Added - Add a new fiat onboarding option via MoonPay ([#13934](https://github.com/MetaMask/metamask-extension/pull/13934)) @@ -2885,7 +2889,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.13.0...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.14.1...HEAD +[10.14.1]: https://github.com/MetaMask/metamask-extension/compare/v10.13.0...v10.14.1 [10.13.0]: https://github.com/MetaMask/metamask-extension/compare/v10.12.4...v10.13.0 [10.12.4]: https://github.com/MetaMask/metamask-extension/compare/v10.12.3...v10.12.4 [10.12.3]: https://github.com/MetaMask/metamask-extension/compare/v10.12.2...v10.12.3 diff --git a/package.json b/package.json index e8e4a9655..a6a062f61 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.13.0", + "version": "10.14.1", "private": true, "repository": { "type": "git", From fefe9401a19dd7c3c6c0c71ed0fdb04830c05fdc Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 4 May 2022 05:06:33 -1000 Subject: [PATCH 04/16] build - update bify-module-groups for build determinism (#14610) --- package.json | 2 +- yarn.lock | 30 ++++-------------------------- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index a6a062f61..b59f069a1 100644 --- a/package.json +++ b/package.json @@ -277,7 +277,7 @@ "@typescript-eslint/parser": "^4.20.0", "addons-linter": "1.14.0", "babelify": "^10.0.0", - "bify-module-groups": "^1.0.0", + "bify-module-groups": "^2.0.0", "brfs": "^2.0.2", "browser-util-inspect": "^0.2.0", "browserify": "^16.5.1", diff --git a/yarn.lock b/yarn.lock index f3fcb1459..a25b002bf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6654,24 +6654,14 @@ better-opn@^2.1.1: dependencies: open "^7.0.3" -bify-module-groups@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/bify-module-groups/-/bify-module-groups-1.0.0.tgz#6fba8f96a8b0f9e8f0b04035650fd56249b6119d" - integrity sha512-JAAkE9L5vZoALCEqawXipQNlDn3D0nUyjt0cHgRXj0Kce2RNLQsBxA6wTmnYpQDna6g6VVyC8IUi3n02ppmbhA== +bify-module-groups@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/bify-module-groups/-/bify-module-groups-2.0.0.tgz#b629b0028db855b7a587d932ea3af7ed4a69dca9" + integrity sha512-9hkVBhhjO5ycUGlUT6KW109gOgsmnrDH+vMjPFFkY9oCiP397p5o4wruXZqyI9ZA1p8hA5egoKBoh3GwbKiM4g== dependencies: - bify-packagedata-stream "1.0.0" pump "^3.0.0" through2 "^3.0.1" -bify-packagedata-stream@1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/bify-packagedata-stream/-/bify-packagedata-stream-1.0.0.tgz#a6dbdcba64f9bf1c87bdc02ba9586fff7b94ccb3" - integrity sha512-ckOCceDpAOySFrt89saOShpVbP/iQbmZeWlYSxZV2e3HPTPhcd3JSudMJZhpsihQTyZut39efDo4+8aOb4vo2w== - dependencies: - module-name-from-path "git+https://git@github.com/kumavis/module-name-from-path.git" - resolve-package-path "^1.2.7" - through2 "^3.0.0" - big-integer@1.6.36: version "1.6.36" resolved "https://registry.yarnpkg.com/big-integer/-/big-integer-1.6.36.tgz#78631076265d4ae3555c04f85e7d9d2f3a071a36" @@ -19277,10 +19267,6 @@ module-lookup-amd@^7.0.0: requirejs "^2.3.5" requirejs-config-file "^4.0.0" -"module-name-from-path@git+https://git@github.com/kumavis/module-name-from-path.git": - version "1.0.4" - resolved "git+https://git@github.com/kumavis/module-name-from-path.git#fd9c592663a1af6cc48b1be7b8045ea547fca79a" - module-not-found-error@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/module-not-found-error/-/module-not-found-error-1.0.1.tgz#cf8b4ff4f29640674d6cdd02b0e3bc523c2bbdc0" @@ -23961,14 +23947,6 @@ resolve-options@^1.1.0: dependencies: value-or-function "^3.0.0" -resolve-package-path@^1.2.7: - version "1.2.7" - resolved "https://registry.yarnpkg.com/resolve-package-path/-/resolve-package-path-1.2.7.tgz#2a7bc37ad96865e239330e3102c31322847e652e" - integrity sha512-fVEKHGeK85bGbVFuwO9o1aU0n3vqQGrezPc51JGu9UTXpFQfWq5qCeKxyaRUSvephs+06c5j5rPq/dzHGEo8+Q== - dependencies: - path-root "^0.1.1" - resolve "^1.10.0" - resolve-pathname@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/resolve-pathname/-/resolve-pathname-3.0.0.tgz#99d02224d3cf263689becbb393bc560313025dcd" From a58faa13a35e4bf6c014389dce112745ef83248d Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 4 May 2022 12:57:38 -0230 Subject: [PATCH 05/16] Version v10.14.2 This version includes a build system fix that ensures our builds are deterministic. --- CHANGELOG.md | 8 +++++++- package.json | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e9550f1c..4980628ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.14.2] +### Fixed +- Make build deterministic (#14610) + - The ordering of modules within each bundle was non-deterministic before this change. We fixed this to comply with Firefox store policies. + ## [10.14.1] ### Changed - This version was used to rollback from v10.14.0 to v10.13.0. @@ -2889,7 +2894,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.14.1...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.14.2...HEAD +[10.14.2]: https://github.com/MetaMask/metamask-extension/compare/v10.14.1...v10.14.2 [10.14.1]: https://github.com/MetaMask/metamask-extension/compare/v10.13.0...v10.14.1 [10.13.0]: https://github.com/MetaMask/metamask-extension/compare/v10.12.4...v10.13.0 [10.12.4]: https://github.com/MetaMask/metamask-extension/compare/v10.12.3...v10.12.4 diff --git a/package.json b/package.json index b59f069a1..549f139bf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.14.1", + "version": "10.14.2", "private": true, "repository": { "type": "git", From 8a14504b6381bc62af9423a1be6d6e53aa5d5cf3 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Sat, 14 May 2022 21:03:06 -0230 Subject: [PATCH 06/16] Version v10.14.5 This version is equivalent to v10.14.2. This release is just intended to fix build configuration issues. --- CHANGELOG.md | 17 ++++++++++++++++- package.json | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4980628ee..0da81e0ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.14.5] +### Fixed +- This release was deployed to fix a configuration issue. + +## [10.14.4] +### Fixed +- This release was deployed to fix a configuration issue. + +## [10.14.3] +### Fixed +- This release was deployed to fix a configuration issue. + ## [10.14.2] ### Fixed - Make build deterministic (#14610) @@ -2894,7 +2906,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.14.2...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.14.5...HEAD +[10.14.5]: https://github.com/MetaMask/metamask-extension/compare/v10.14.4...v10.14.5 +[10.14.4]: https://github.com/MetaMask/metamask-extension/compare/v10.14.3...v10.14.4 +[10.14.3]: https://github.com/MetaMask/metamask-extension/compare/v10.14.2...v10.14.3 [10.14.2]: https://github.com/MetaMask/metamask-extension/compare/v10.14.1...v10.14.2 [10.14.1]: https://github.com/MetaMask/metamask-extension/compare/v10.13.0...v10.14.1 [10.13.0]: https://github.com/MetaMask/metamask-extension/compare/v10.12.4...v10.13.0 diff --git a/package.json b/package.json index 549f139bf..ccc788828 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.14.2", + "version": "10.14.5", "private": true, "repository": { "type": "git", From 7199d9c56775111f85225fe15297e47de8e2bc96 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 5 May 2022 19:58:48 -0230 Subject: [PATCH 07/16] Use externally hosted phishing warning page An externally hosted phishing warning page is now used rather than the built-in phishing warning page.The phishing page warning URL is set via configuration file or environment variable. The default URL is either the expected production URL or `http://localhost:9999/` for e2e testing environments. The new external phishing page includes a design change when it is loaded within an iframe. In that case it now shows a condensed message, and prompts the user to open the full warning page in a new tab to see more details or bypass the warning. This is to prevent a clickjacking attack from safelisting a site without user consent. The new external phishing page also includes a simple caching service worker to ensure it continues to work offline (or if our hosting goes offline), as long as the user has successfully loaded the page at least once. We also load the page temporarily during the extension startup process to trigger the service worker installation. The old phishing page and all related lines have been removed. The property `web_accessible_resources` has also been removed from the manifest. The only entry apart from the phishing page was `inpage.js`, and we don't need that to be web accessible anymore because we inject the script inline into each page rather than loading the file directly. New e2e tests have been added to cover more phishing warning page functionality, including the "safelist" action and the "iframe" case. --- app/manifest/_base.json | 3 +- app/phishing.html | 150 ------------------ app/scripts/background.js | 86 ++++++++++ app/scripts/contentscript.js | 61 ++++++- app/scripts/lib/util.js | 2 +- app/scripts/lib/util.test.js | 7 - app/scripts/metamask-controller.js | 31 +++- app/scripts/phishing-detect.js | 52 ------ development/build/scripts.js | 67 +++++--- development/build/static.js | 4 - development/sourcemap-validator.js | 1 - package.json | 1 + test/e2e/helpers.js | 11 +- .../index.html | 19 +++ test/e2e/mock-page-with-iframe/index.html | 11 ++ test/e2e/phishing-warning-page-server.js | 58 +++++++ test/e2e/tests/phishing-detection.spec.js | 80 +++++++++- test/e2e/webdriver/driver.js | 4 + yarn.lock | 21 ++- 19 files changed, 413 insertions(+), 256 deletions(-) delete mode 100644 app/phishing.html delete mode 100644 app/scripts/phishing-detect.js create mode 100644 test/e2e/mock-page-with-disallowed-iframe/index.html create mode 100644 test/e2e/mock-page-with-iframe/index.html create mode 100644 test/e2e/phishing-warning-page-server.js diff --git a/app/manifest/_base.json b/app/manifest/_base.json index c2b20b4db..87bed959d 100644 --- a/app/manifest/_base.json +++ b/app/manifest/_base.json @@ -72,6 +72,5 @@ "*://*.eth/", "notifications" ], - "short_name": "__MSG_appName__", - "web_accessible_resources": ["inpage.js", "phishing.html"] + "short_name": "__MSG_appName__" } diff --git a/app/phishing.html b/app/phishing.html deleted file mode 100644 index e854d79fa..000000000 --- a/app/phishing.html +++ /dev/null @@ -1,150 +0,0 @@ - - - - MetaMask Phishing Detection - - - - - - - - - - -
-
- MetaMask Logo -

- - MetaMask Phishing Detection -

-
-
-

- This domain is currently on the MetaMask domain warning list. This - means that based on information available to us, MetaMask believes - this domain could currently compromise your security and, as an added - safety feature, MetaMask has restricted access to the site. To - override this, please read the rest of this warning for instructions - on how to continue at your own risk. -

-

- There are many reasons sites can appear on our warning list, and our - warning list compiles from other widely used industry lists. Such - reasons can include known fraud or security risks, such as domains - that test positive on the - Ethereum Phishing Detector. Domains on these warning lists may include outright malicious - websites and legitimate websites that have been compromised by a - malicious actor. -

-

- To read more about this site - please search for the domain on CryptoScamDB. -

-

- Note that this warning list is compiled on a voluntary basis. This - list may be inaccurate or incomplete. Just because a domain does not - appear on this list is not an implicit guarantee of that domain's - safety. As always, your transactions are your own responsibility. If - you wish to interact with any domain on our warning list, you can do - so by continuing at your own risk. -

-

- If you think this domain is incorrectly flagged or if a blocked - legitimate website has resolved its security issues, - please file an issue. -

-
-
- - diff --git a/app/scripts/background.js b/app/scripts/background.js index ae85d781b..5e3cd7770 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -67,6 +67,12 @@ if (inTest || process.env.METAMASK_DEBUG) { global.metamaskGetState = localStore.get.bind(localStore); } +const phishingPageUrl = new URL(process.env.PHISHING_PAGE_URL); + +const ONE_SECOND_IN_MILLISECONDS = 1_000; +// Timeout for initializing phishing warning page. +const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; + // initialization flow initialize().catch(log.error); @@ -134,9 +140,76 @@ async function initialize() { const initState = await loadStateFromPersistence(); const initLangCode = await getFirstPreferredLangCode(); await setupController(initState, initLangCode); + await loadPhishingWarningPage(); log.info('MetaMask initialization complete.'); } +/** + * An error thrown if the phishing warning page takes too long to load. + */ +class PhishingWarningPageTimeoutError extends Error { + constructor() { + super('Timeout failed'); + } +} + +/** + * Load the phishing warning page temporarily to ensure the service + * worker has been registered, so that the warning page works offline. + */ +async function loadPhishingWarningPage() { + let iframe; + try { + const extensionStartupPhishingPageUrl = new URL( + process.env.PHISHING_PAGE_URL, + ); + // The `extensionStartup` hash signals to the phishing warning page that it should not bother + // setting up streams for user interaction. Otherwise this page load would cause a console + // error. + extensionStartupPhishingPageUrl.hash = '#extensionStartup'; + + iframe = window.document.createElement('iframe'); + iframe.setAttribute('src', extensionStartupPhishingPageUrl.href); + iframe.setAttribute('sandbox', 'allow-scripts allow-same-origin'); + + // Create "deferred Promise" to allow passing resolve/reject to event handlers + let deferredResolve; + let deferredReject; + const loadComplete = new Promise((resolve, reject) => { + deferredResolve = resolve; + deferredReject = reject; + }); + + // The load event is emitted once loading has completed, even if the loading failed. + // If loading failed we can't do anything about it, so we don't need to check. + iframe.addEventListener('load', deferredResolve); + + // This step initiates the page loading. + window.document.body.appendChild(iframe); + + // This timeout ensures that this iframe gets cleaned up in a reasonable + // timeframe, and ensures that the "initialization complete" message + // doesn't get delayed too long. + setTimeout( + () => deferredReject(new PhishingWarningPageTimeoutError()), + PHISHING_WARNING_PAGE_TIMEOUT, + ); + await loadComplete; + } catch (error) { + if (error instanceof PhishingWarningPageTimeoutError) { + console.warn( + 'Phishing warning page timeout; page not guaraneteed to work offline.', + ); + } else { + console.error('Failed to initialize phishing warning page', error); + } + } finally { + if (iframe) { + iframe.remove(); + } + } +} + // // State and Persistence // @@ -362,6 +435,10 @@ function setupController(initState, initLangCode) { remotePort.sender.origin === `chrome-extension://${browser.runtime.id}`; } + const senderUrl = remotePort.sender?.url + ? new URL(remotePort.sender.url) + : null; + if (isMetaMaskInternalProcess) { const portStream = new PortStream(remotePort); // communication with popup @@ -406,6 +483,15 @@ function setupController(initState, initLangCode) { ); }); } + } else if ( + senderUrl && + senderUrl.origin === phishingPageUrl.origin && + senderUrl.pathname === phishingPageUrl.pathname + ) { + const portStream = new PortStream(remotePort); + controller.setupPhishingCommunication({ + connectionStream: portStream, + }); } else { if (remotePort.sender && remotePort.sender.tab && remotePort.sender.url) { const tabId = remotePort.sender.tab.id; diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 037f3de14..57850a535 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -17,8 +17,13 @@ const inpageContent = fs.readFileSync( const inpageSuffix = `//# sourceURL=${browser.runtime.getURL('inpage.js')}\n`; const inpageBundle = inpageContent + inpageSuffix; +// contexts const CONTENT_SCRIPT = 'metamask-contentscript'; const INPAGE = 'metamask-inpage'; +const PHISHING_WARNING_PAGE = 'metamask-phishing-warning-page'; + +// stream channels +const PHISHING_SAFELIST = 'metamask-phishing-safelist'; const PROVIDER = 'metamask-provider'; // TODO:LegacyProvider: Delete @@ -27,7 +32,14 @@ const LEGACY_INPAGE = 'inpage'; const LEGACY_PROVIDER = 'provider'; const LEGACY_PUBLIC_CONFIG = 'publicConfig'; -if (shouldInjectProvider()) { +const phishingPageUrl = new URL(process.env.PHISHING_PAGE_URL); + +if ( + window.location.origin === phishingPageUrl.origin && + window.location.pathname === phishingPageUrl.pathname +) { + setupPhishingStream(); +} else if (shouldInjectProvider()) { injectScript(inpageBundle); setupStreams(); } @@ -50,6 +62,47 @@ function injectScript(content) { } } +async function setupPhishingStream() { + // the transport-specific streams for communication between inpage and background + const pageStream = new WindowPostMessageStream({ + name: CONTENT_SCRIPT, + target: PHISHING_WARNING_PAGE, + }); + const extensionPort = browser.runtime.connect({ name: CONTENT_SCRIPT }); + const extensionStream = new PortStream(extensionPort); + + // create and connect channel muxers + // so we can handle the channels individually + const pageMux = new ObjectMultiplex(); + pageMux.setMaxListeners(25); + const extensionMux = new ObjectMultiplex(); + extensionMux.setMaxListeners(25); + + pump(pageMux, pageStream, pageMux, (err) => + logStreamDisconnectWarning('MetaMask Inpage Multiplex', err), + ); + pump(extensionMux, extensionStream, extensionMux, (err) => { + logStreamDisconnectWarning('MetaMask Background Multiplex', err); + window.postMessage( + { + target: PHISHING_WARNING_PAGE, // the post-message-stream "target" + data: { + // this object gets passed to obj-multiplex + name: PHISHING_SAFELIST, // the obj-multiplex channel name + data: { + jsonrpc: '2.0', + method: 'METAMASK_STREAM_FAILURE', + }, + }, + }, + window.location.origin, + ); + }); + + // forward communication across inpage-background for these channels only + forwardTrafficBetweenMuxes(PHISHING_SAFELIST, pageMux, extensionMux); +} + /** * Sets up two-way communication streams between the * browser extension and local per-page browser context. @@ -300,9 +353,9 @@ function blockedDomainCheck() { * Redirects the current page to a phishing information page */ function redirectToPhishingWarning() { - console.debug('MetaMask: Routing to Phishing Warning component.'); - const extensionURL = browser.runtime.getURL('phishing.html'); - window.location.href = `${extensionURL}#${querystring.stringify({ + console.debug('MetaMask: Routing to Phishing Warning page.'); + const baseUrl = process.env.PHISHING_PAGE_URL; + window.location.href = `${baseUrl}#${querystring.stringify({ hostname: window.location.hostname, href: window.location.href, })}`; diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index 9800666af..b7fdf0521 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -27,7 +27,7 @@ const getEnvironmentTypeMemo = memoize((url) => { const parsedUrl = new URL(url); if (parsedUrl.pathname === '/popup.html') { return ENVIRONMENT_TYPE_POPUP; - } else if (['/home.html', '/phishing.html'].includes(parsedUrl.pathname)) { + } else if (['/home.html'].includes(parsedUrl.pathname)) { return ENVIRONMENT_TYPE_FULLSCREEN; } else if (parsedUrl.pathname === '/notification.html') { return ENVIRONMENT_TYPE_NOTIFICATION; diff --git a/app/scripts/lib/util.test.js b/app/scripts/lib/util.test.js index 62c33b5a5..0892a4e3d 100644 --- a/app/scripts/lib/util.test.js +++ b/app/scripts/lib/util.test.js @@ -34,13 +34,6 @@ describe('app utils', () => { expect(environmentType).toStrictEqual(ENVIRONMENT_TYPE_FULLSCREEN); }); - it('should return fullscreen type for phishing.html', () => { - const environmentType = getEnvironmentType( - 'http://extension-id/phishing.html', - ); - expect(environmentType).toStrictEqual(ENVIRONMENT_TYPE_FULLSCREEN); - }); - it('should return background type', () => { const environmentType = getEnvironmentType( 'http://extension-id/_generated_background_page.html', diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e5e8493fd..c307068d2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -145,6 +145,9 @@ export const METAMASK_CONTROLLER_EVENTS = { APPROVAL_STATE_CHANGE: 'ApprovalController:stateChange', }; +// stream channels +const PHISHING_SAFELIST = 'metamask-phishing-safelist'; + export default class MetamaskController extends EventEmitter { /** * @param {Object} opts @@ -1371,7 +1374,6 @@ export default class MetamaskController extends EventEmitter { ), markPasswordForgotten: this.markPasswordForgotten.bind(this), unMarkPasswordForgotten: this.unMarkPasswordForgotten.bind(this), - safelistPhishingDomain: this.safelistPhishingDomain.bind(this), getRequestAccountTabIds: this.getRequestAccountTabIds, getOpenMetamaskTabsIds: this.getOpenMetamaskTabsIds, markNotificationPopupAsAutomaticallyClosed: () => @@ -3152,6 +3154,33 @@ export default class MetamaskController extends EventEmitter { ); } + /** + * Used to create a multiplexed stream for connecting to the phishing warning page. + * + * @param options - Options bag. + * @param {ReadableStream} options.connectionStream - The Duplex stream to connect to. + */ + setupPhishingCommunication({ connectionStream }) { + const { usePhishDetect } = this.preferencesController.store.getState(); + + if (!usePhishDetect) { + return; + } + + // setup multiplexing + const mux = setupMultiplex(connectionStream); + const phishingStream = mux.createStream(PHISHING_SAFELIST); + + // set up postStream transport + phishingStream.on( + 'data', + createMetaRPCHandler( + { safelistPhishingDomain: this.safelistPhishingDomain.bind(this) }, + phishingStream, + ), + ); + } + /** * Called when we detect a suspicious domain. Requests the browser redirects * to our anti-phishing page. diff --git a/app/scripts/phishing-detect.js b/app/scripts/phishing-detect.js deleted file mode 100644 index f926b234d..000000000 --- a/app/scripts/phishing-detect.js +++ /dev/null @@ -1,52 +0,0 @@ -import querystring from 'querystring'; -import PortStream from 'extension-port-stream'; -import browser from 'webextension-polyfill'; -import createRandomId from '../../shared/modules/random-id'; -import { setupMultiplex } from './lib/stream-utils'; -import { getEnvironmentType } from './lib/util'; -import ExtensionPlatform from './platforms/extension'; - -document.addEventListener('DOMContentLoaded', start); - -function start() { - const hash = window.location.hash.substring(1); - const suspect = querystring.parse(hash); - - const newIssueLink = document.getElementById('new-issue-link'); - const newIssueUrl = `https://github.com/MetaMask/eth-phishing-detect/issues/new`; - const newIssueParams = `?title=[Legitimate%20Site%20Blocked]%20${encodeURIComponent( - suspect.hostname, - )}&body=${encodeURIComponent(suspect.href)}`; - newIssueLink.href = `${newIssueUrl}${newIssueParams}`; - - global.platform = new ExtensionPlatform(); - - const extensionPort = browser.runtime.connect({ - name: getEnvironmentType(), - }); - const connectionStream = new PortStream(extensionPort); - const mx = setupMultiplex(connectionStream); - const backgroundConnection = mx.createStream('controller'); - const continueLink = document.getElementById('unsafe-continue'); - continueLink.addEventListener('click', () => { - backgroundConnection.write({ - jsonrpc: '2.0', - method: 'safelistPhishingDomain', - params: [suspect.hostname], - id: createRandomId(), - }); - const redirectTarget = new URL(suspect.href, window.location.href); - // validate redirect url - const invalidProtocol = !['https:', 'http:'].includes( - redirectTarget.protocol, - ); - // if in valid, show warning and abort - if (invalidProtocol) { - // we intentionally dont display to the user any potential attacker-written content here - console.error(`Invalid redirect url.`); - return; - } - // use the validated url instance - window.location.href = redirectTarget.href; - }); -} diff --git a/development/build/scripts.js b/development/build/scripts.js index f942c4c1b..8e2a3864e 100644 --- a/development/build/scripts.js +++ b/development/build/scripts.js @@ -34,6 +34,7 @@ const metamaskrc = require('rc')('metamask', { INFURA_PROD_PROJECT_ID: process.env.INFURA_PROD_PROJECT_ID, ONBOARDING_V2: process.env.ONBOARDING_V2, COLLECTIBLES_V1: process.env.COLLECTIBLES_V1, + PHISHING_PAGE_URL: process.env.PHISHING_PAGE_URL, TOKEN_DETECTION_V2: process.env.TOKEN_DETECTION_V2, SEGMENT_HOST: process.env.SEGMENT_HOST, SEGMENT_WRITE_KEY: process.env.SEGMENT_WRITE_KEY, @@ -133,6 +134,48 @@ function getSegmentWriteKey({ buildType, environment }) { throw new Error(`Invalid build type: '${buildType}'`); } +/** + * Get the URL for the phishing warning page, if it has been set. + * + * @param options0 + * @param options0.testing + * @returns {string} The URL for the phishing warning page, or `undefined` if no URL is set. + */ +function getPhishingWarningPageUrl({ testing }) { + let phishingWarningPageUrl = metamaskrc.PHISHING_PAGE_URL; + + if (!phishingWarningPageUrl) { + phishingWarningPageUrl = testing + ? 'http://localhost:9999/' + : 'https://metamask.github.io/phishing-warning/v1.1.0/'; + } + + // We add a hash/fragment to the URL dynamically, so we need to ensure it + // has a valid pathname to append a hash to. + const normalizedUrl = phishingWarningPageUrl.endsWith('/') + ? phishingWarningPageUrl + : `${phishingWarningPageUrl}/`; + + let phishingWarningPageUrlObject; + try { + // eslint-disable-next-line no-new + phishingWarningPageUrlObject = new URL(normalizedUrl); + } catch (error) { + throw new Error( + `Invalid phishing warning page URL: '${normalizedUrl}'`, + error, + ); + } + if (phishingWarningPageUrlObject.hash) { + // The URL fragment must be set dynamically + throw new Error( + `URL fragment not allowed in phishing warning page URL: '${normalizedUrl}'`, + ); + } + + return normalizedUrl; +} + const noopWriteStream = through.obj((_file, _fileEncoding, callback) => callback(), ); @@ -216,11 +259,6 @@ function createScriptTasks({ createTaskForBundleSentry({ devMode, testing }), ); - const phishingDetectSubtask = createTask( - `${taskPrefix}:phishing-detect`, - createTaskForBundlePhishingDetect({ devMode, testing }), - ); - // task for initiating browser livereload const initiateLiveReload = async () => { if (devMode) { @@ -243,7 +281,6 @@ function createScriptTasks({ contentscriptSubtask, disableConsoleSubtask, installSentrySubtask, - phishingDetectSubtask, ].map((subtask) => runInChildProcess(subtask, { buildType, @@ -290,23 +327,6 @@ function createScriptTasks({ }); } - function createTaskForBundlePhishingDetect({ devMode, testing }) { - const label = 'phishing-detect'; - return createNormalBundle({ - buildType, - browserPlatforms, - destFilepath: `${label}.js`, - devMode, - entryFilepath: `./app/scripts/${label}.js`, - ignoredFiles, - label, - testing, - policyOnly, - shouldLintFenceFiles, - version, - }); - } - // the "contentscript" bundle contains the "inpage" bundle function createTaskForBundleContentscript({ devMode, testing }) { const inpage = 'inpage'; @@ -818,6 +838,7 @@ function getEnvironmentVariables({ buildType, devMode, testing, version }) { METAMASK_BUILD_TYPE: buildType, NODE_ENV: devMode ? ENVIRONMENT.DEVELOPMENT : ENVIRONMENT.PRODUCTION, IN_TEST: testing, + PHISHING_PAGE_URL: getPhishingWarningPageUrl({ testing }), PUBNUB_SUB_KEY: process.env.PUBNUB_SUB_KEY || '', PUBNUB_PUB_KEY: process.env.PUBNUB_PUB_KEY || '', CONF: devMode ? metamaskrc : {}, diff --git a/development/build/static.js b/development/build/static.js index 7efde5a72..d1f024fbc 100644 --- a/development/build/static.js +++ b/development/build/static.js @@ -174,10 +174,6 @@ function getCopyTargets(shouldIncludeLockdown) { src: require.resolve('@lavamoat/lavapack/src/runtime.js'), dest: `runtime-lavamoat.js`, }, - { - src: `./app/phishing.html`, - dest: `phishing.html`, - }, ]; const languageTags = new Set(); diff --git a/development/sourcemap-validator.js b/development/sourcemap-validator.js index 01861e229..0531a03ce 100644 --- a/development/sourcemap-validator.js +++ b/development/sourcemap-validator.js @@ -24,7 +24,6 @@ async function start() { `common-0.js`, `background-0.js`, `ui-0.js`, - 'phishing-detect.js', // `contentscript.js`, skipped because the validator is erroneously sampling the inlined `inpage.js` script `inpage.js`, ]; diff --git a/package.json b/package.json index ccc788828..fd1117e5e 100644 --- a/package.json +++ b/package.json @@ -252,6 +252,7 @@ "@metamask/eslint-config-nodejs": "^9.0.0", "@metamask/eslint-config-typescript": "^9.0.1", "@metamask/forwarder": "^1.1.0", + "@metamask/phishing-warning": "^1.0.0", "@metamask/test-dapp": "^5.0.0", "@sentry/cli": "^1.58.0", "@storybook/addon-a11y": "^6.3.12", diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index a6f7b3c7b..1b611b989 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -6,6 +6,7 @@ const enLocaleMessages = require('../../app/_locales/en/messages.json'); const { setupMocking } = require('./mock-e2e'); const Ganache = require('./ganache'); const FixtureServer = require('./fixture-server'); +const PhishingWarningPageServer = require('./phishing-warning-page-server'); const { buildWebDriver } = require('./webdriver'); const { ensureXServerIsRunning } = require('./x-server'); @@ -27,6 +28,7 @@ async function withFixtures(options, testSuite) { title, failOnConsoleError = true, dappPath = undefined, + dappPaths, testSpecificMock = function () { // do nothing. }, @@ -38,6 +40,7 @@ async function withFixtures(options, testSuite) { let secondaryGanacheServer; let numberOfDapps = dapp ? 1 : 0; const dappServer = []; + const phishingPageServer = new PhishingWarningPageServer(); let webDriver; let failed = false; @@ -55,14 +58,15 @@ async function withFixtures(options, testSuite) { } await fixtureServer.start(); await fixtureServer.loadState(path.join(__dirname, 'fixtures', fixtures)); + await phishingPageServer.start(); if (dapp) { if (dappOptions?.numberOfDapps) { numberOfDapps = dappOptions.numberOfDapps; } for (let i = 0; i < numberOfDapps; i++) { let dappDirectory; - if (dappPath) { - dappDirectory = path.resolve(__dirname, dappPath); + if (dappPath || (dappPaths && dappPaths[i])) { + dappDirectory = path.resolve(__dirname, dappPath || dappPaths[i]); } else { dappDirectory = path.resolve( __dirname, @@ -146,6 +150,9 @@ async function withFixtures(options, testSuite) { } } } + if (phishingPageServer.isRunning()) { + await phishingPageServer.quit(); + } await mockServer.stop(); } } diff --git a/test/e2e/mock-page-with-disallowed-iframe/index.html b/test/e2e/mock-page-with-disallowed-iframe/index.html new file mode 100644 index 000000000..c6309677e --- /dev/null +++ b/test/e2e/mock-page-with-disallowed-iframe/index.html @@ -0,0 +1,19 @@ + + + + Mock E2E Phishing Page + + + +
Hello
+