From ecaa235b5e3331defab75dad72593951fdf37790 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 2 Aug 2017 14:26:10 -0700 Subject: [PATCH] phishing detection - move phishing detection into contentscript and metamask controller --- app/manifest.json | 9 ----- app/scripts/background.js | 34 +++---------------- app/scripts/blacklister.js | 14 -------- app/scripts/contentscript.js | 22 ++++++++---- app/scripts/lib/inpage-provider.js | 3 ++ app/scripts/lib/is-phish.js | 6 ++-- app/scripts/metamask-controller.js | 26 ++++++++++++-- gulpfile.js | 1 - package.json | 2 +- ...ter-test.js => phishing-detection-test.js} | 2 +- 10 files changed, 52 insertions(+), 67 deletions(-) delete mode 100644 app/scripts/blacklister.js rename test/unit/{blacklister-test.js => phishing-detection-test.js} (96%) diff --git a/app/manifest.json b/app/manifest.json index 591a07d0d..1eaf6f26a 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -52,15 +52,6 @@ ], "run_at": "document_start", "all_frames": true - }, - { - "run_at": "document_start", - "matches": [ - "http://*/*", - "https://*/*" - ], - "js": ["scripts/blacklister.js"], - "all_frames": true } ], "permissions": [ diff --git a/app/scripts/background.js b/app/scripts/background.js index bc0fbdc37..a235afff3 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -11,7 +11,6 @@ const NotificationManager = require('./lib/notification-manager.js') const MetamaskController = require('./metamask-controller') const extension = require('extensionizer') const firstTimeState = require('./first-time-state') -const isPhish = require('./lib/is-phish') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' @@ -91,16 +90,12 @@ function setupController (initState) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { - if (remotePort.name === 'blacklister') { - return checkBlacklist(remotePort) - } - - var isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' - var portStream = new PortStream(remotePort) + const isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' + const portStream = new PortStream(remotePort) if (isMetaMaskInternalProcess) { // communication with popup popupIsOpen = popupIsOpen || (remotePort.name === 'popup') - controller.setupTrustedCommunication(portStream, 'MetaMask', remotePort.name) + controller.setupTrustedCommunication(portStream, 'MetaMask') // record popup as closed if (remotePort.name === 'popup') { endOfStream(portStream, () => { @@ -109,7 +104,7 @@ function setupController (initState) { } } else { // communication with page - var originDomain = urlUtil.parse(remotePort.sender.url).hostname + const originDomain = urlUtil.parse(remotePort.sender.url).hostname controller.setupUntrustedCommunication(portStream, originDomain) } } @@ -140,27 +135,6 @@ function setupController (initState) { return Promise.resolve() } -// Listen for new pages and return if blacklisted: -function checkBlacklist (port) { - const handler = handleNewPageLoad.bind(null, port) - port.onMessage.addListener(handler) - setTimeout(() => { - port.onMessage.removeListener(handler) - }, 30000) -} - -function handleNewPageLoad (port, message) { - const { pageLoaded } = message - if (!pageLoaded || !global.metamaskController) return - - const state = global.metamaskController.getState() - const updatedBlacklist = state.blacklist - - if (isPhish({ updatedBlacklist, hostname: pageLoaded })) { - port.postMessage({ 'blacklist': pageLoaded }) - } -} - // // Etc... // diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js deleted file mode 100644 index 37751b595..000000000 --- a/app/scripts/blacklister.js +++ /dev/null @@ -1,14 +0,0 @@ -const extension = require('extensionizer') - -var port = extension.runtime.connect({name: 'blacklister'}) -port.postMessage({ 'pageLoaded': window.location.hostname }) -port.onMessage.addListener(redirectIfBlacklisted) - -function redirectIfBlacklisted (response) { - const { blacklist } = response - const host = window.location.hostname - if (blacklist && blacklist === host) { - window.location.href = 'https://metamask.io/phishing.html' - } -} - diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 291b922e8..6fde0edcd 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -37,28 +37,33 @@ function setupInjection () { function setupStreams () { // setup communication to page and plugin - var pageStream = new LocalMessageDuplexStream({ + const pageStream = new LocalMessageDuplexStream({ name: 'contentscript', target: 'inpage', }) pageStream.on('error', console.error) - var pluginPort = extension.runtime.connect({name: 'contentscript'}) - var pluginStream = new PortStream(pluginPort) + const pluginPort = extension.runtime.connect({ name: 'contentscript' }) + const pluginStream = new PortStream(pluginPort) pluginStream.on('error', console.error) // forward communication plugin->inpage pageStream.pipe(pluginStream).pipe(pageStream) // setup local multistream channels - var mx = ObjectMultiplex() + const mx = ObjectMultiplex() mx.on('error', console.error) mx.pipe(pageStream).pipe(mx) + mx.pipe(pluginStream).pipe(mx) // connect ping stream - var pongStream = new PongStream({ objectMode: true }) + const pongStream = new PongStream({ objectMode: true }) pongStream.pipe(mx.createStream('pingpong')).pipe(pongStream) - // ignore unused channels (handled by background) + // connect phishing warning stream + const phishingStream = mx.createStream('phishing') + phishingStream.once('data', redirectToPhishingWarning) + + // ignore unused channels (handled by background, inpage) mx.ignoreStream('provider') mx.ignoreStream('publicConfig') } @@ -88,3 +93,8 @@ function suffixCheck () { } return true } + +function redirectToPhishingWarning () { + console.log('MetaMask - redirecting to phishing warning') + window.location.href = 'https://metamask.io/phishing.html' +} diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index 8b8623974..fd032a673 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -26,6 +26,9 @@ function MetamaskInpageProvider (connectionStream) { (err) => logStreamDisconnectWarning('MetaMask PublicConfigStore', err) ) + // ignore phishing warning message (handled elsewhere) + multiStream.ignoreStream('phishing') + // connect to async provider const asyncProvider = self.asyncProvider = new StreamProvider() pipe( diff --git a/app/scripts/lib/is-phish.js b/app/scripts/lib/is-phish.js index 68c09e4ac..21c68b0d6 100644 --- a/app/scripts/lib/is-phish.js +++ b/app/scripts/lib/is-phish.js @@ -9,15 +9,15 @@ const LEVENSHTEIN_CHECKS = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'meta // credit to @sogoiii and @409H for their help! // Return a boolean on whether or not a phish is detected. -function isPhish({ hostname, updatedBlacklist = null }) { +function isPhish({ hostname, blacklist }) { var strCurrentTab = hostname // check if the domain is part of the whitelist. if (whitelistedDomains && whitelistedDomains.includes(strCurrentTab)) { return false } // Allow updating of blacklist: - if (updatedBlacklist) { - blacklistedDomains = blacklistedDomains.concat(updatedBlacklist) + if (blacklist) { + blacklistedDomains = blacklistedDomains.concat(blacklist) } // check if the domain is part of the blacklist. diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 11dcde2c1..28c35a13d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -23,6 +23,7 @@ const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') const getBuyEthUrl = require('./lib/buy-eth-url') +const checkForPhishing = require('./lib/is-phish') const debounce = require('debounce') const version = require('../manifest.json').version @@ -326,8 +327,15 @@ module.exports = class MetamaskController extends EventEmitter { } setupUntrustedCommunication (connectionStream, originDomain) { + // Check if new connection is blacklisted + if (this.isHostBlacklisted(originDomain)) { + console.log('MetaMask - sending phishing warning for', originDomain) + this.sendPhishingWarning(connectionStream, originDomain) + return + } + // setup multiplexing - var mx = setupMultiplex(connectionStream) + const mx = setupMultiplex(connectionStream) // connect features this.setupProviderConnection(mx.createStream('provider'), originDomain) this.setupPublicConfig(mx.createStream('publicConfig')) @@ -335,12 +343,26 @@ module.exports = class MetamaskController extends EventEmitter { setupTrustedCommunication (connectionStream, originDomain) { // setup multiplexing - var mx = setupMultiplex(connectionStream) + const mx = setupMultiplex(connectionStream) // connect features this.setupControllerConnection(mx.createStream('controller')) this.setupProviderConnection(mx.createStream('provider'), originDomain) } + // Check if a domain is on our blacklist + isHostBlacklisted (hostname) { + if (!hostname) return false + const { blacklist } = this.getState().blacklist + return checkForPhishing({ blacklist, hostname }) + } + + sendPhishingWarning (connectionStream, hostname) { + const mx = setupMultiplex(connectionStream) + const phishingStream = mx.createStream('phishing') + // phishingStream.write(true) + phishingStream.write({ hostname }) + } + setupControllerConnection (outStream) { const api = this.getApi() const dnode = Dnode(api) diff --git a/gulpfile.js b/gulpfile.js index 53de7a7d9..cc723704a 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -172,7 +172,6 @@ gulp.task('default', ['lint'], function () { const jsFiles = [ 'inpage', 'contentscript', - 'blacklister', 'background', 'popup', ] diff --git a/package.json b/package.json index 10afc8228..40bc78c6f 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,7 @@ "sw-stream": "^2.0.0", "textarea-caret": "^3.0.1", "three.js": "^0.73.2", - "through2": "^2.0.1", + "through2": "^2.0.3", "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.19.1", diff --git a/test/unit/blacklister-test.js b/test/unit/phishing-detection-test.js similarity index 96% rename from test/unit/blacklister-test.js rename to test/unit/phishing-detection-test.js index ce110491c..affcb16c2 100644 --- a/test/unit/blacklister-test.js +++ b/test/unit/phishing-detection-test.js @@ -1,7 +1,7 @@ const assert = require('assert') const isPhish = require('../../app/scripts/lib/is-phish') -describe('blacklister', function () { +describe('phishing detection test', function () { describe('#isPhish', function () { it('should not flag whitelisted values', function () { var result = isPhish({ hostname: 'www.metamask.io' })