From 616a446832f1fccc31a9fce5617024726a5379e2 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 1 Jun 2020 16:24:27 -0700 Subject: [PATCH] Use URL origin instead of hostname for permission domains (#8717) * use URL.origin instead of hostname for tabs and permissions --- app/scripts/background.js | 2 +- app/scripts/metamask-controller.js | 6 ++-- app/scripts/ui.js | 8 ++--- test/e2e/signature-request.spec.js | 2 +- .../controllers/metamask-controller-test.js | 4 +-- ui/app/selectors/tests/permissions.test.js | 34 +++++++++---------- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 9e95d5252..e314086a6 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -384,7 +384,7 @@ function setupController (initState, initLangCode) { if (remotePort.sender && remotePort.sender.tab && remotePort.sender.url) { const tabId = remotePort.sender.tab.id const url = new URL(remotePort.sender.url) - const origin = url.hostname + const { origin } = url remotePort.onMessage.addListener((msg) => { if (msg.data && msg.data.method === 'eth_requestAccounts') { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index cf77ef7ca..f36da612e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1487,7 +1487,7 @@ export default class MetamaskController extends EventEmitter { * @private * @param {*} connectionStream - The duplex stream to the per-page script, * for sending the reload attempt to. - * @param {string} hostname - The URL that triggered the suspicion. + * @param {string} hostname - The hostname that triggered the suspicion. */ sendPhishingWarning (connectionStream, hostname) { const mux = setupMultiplex(connectionStream) @@ -1538,7 +1538,7 @@ export default class MetamaskController extends EventEmitter { setupProviderConnection (outStream, sender, isInternal) { const origin = isInternal ? 'metamask' - : (new URL(sender.url)).hostname + : (new URL(sender.url)).origin let extensionId if (sender.id !== extension.runtime.id) { extensionId = sender.id @@ -1577,7 +1577,7 @@ export default class MetamaskController extends EventEmitter { /** * A method for creating a provider that is safely restricted for the requesting domain. * @param {Object} options - Provider engine options - * @param {string} options.origin - The hostname of the sender + * @param {string} options.origin - The origin of the sender * @param {string} options.location - The full URL of the sender * @param {extensionId} [options.extensionId] - The extension ID of the sender, if the sender is an external extension * @param {tabId} [options.tabId] - The tab ID of the sender - if the sender is within a tab diff --git a/app/scripts/ui.js b/app/scripts/ui.js index 88f3fba07..5a403828d 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -21,7 +21,6 @@ import { EventEmitter } from 'events' import Dnode from 'dnode' import Eth from 'ethjs' import EthQuery from 'eth-query' -import urlUtil from 'url' import launchMetaMaskUi from '../../ui' import StreamProvider from 'web3-stream-provider' import { setupMultiplex } from './lib/stream-utils.js' @@ -95,10 +94,9 @@ async function queryCurrentActiveTab (windowType) { extension.tabs.query({ active: true, currentWindow: true }, (tabs) => { const [activeTab] = tabs const { title, url } = activeTab - const { hostname: origin, protocol } = url ? urlUtil.parse(url) : {} - resolve({ - title, origin, protocol, url, - }) + const { origin, protocol } = url ? new URL(url) : {} + + resolve({ title, origin, protocol, url }) }) }) } diff --git a/test/e2e/signature-request.spec.js b/test/e2e/signature-request.spec.js index d76dbdfc9..76c0ec6a4 100644 --- a/test/e2e/signature-request.spec.js +++ b/test/e2e/signature-request.spec.js @@ -107,7 +107,7 @@ describe('MetaMask', function () { const address = content[1] assert.equal(await title.getText(), 'Signature Request') assert.equal(await name.getText(), 'Ether Mail') - assert.equal(await origin.getText(), '127.0.0.1') + assert.equal(await origin.getText(), 'http://127.0.0.1:8080') assert.equal(await address.getText(), publicAddress.slice(0, 8) + '...' + publicAddress.slice(publicAddress.length - 8)) }) diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 9fd55d071..3201e695c 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -824,7 +824,7 @@ describe('MetaMaskController', function () { 'mock tx params', { ...message, - origin: 'mycrypto.com', + origin: 'http://mycrypto.com', tabId: 456, }, ] @@ -865,7 +865,7 @@ describe('MetaMaskController', function () { 'mock tx params', { ...message, - origin: 'mycrypto.com', + origin: 'http://mycrypto.com', }, ] ) diff --git a/ui/app/selectors/tests/permissions.test.js b/ui/app/selectors/tests/permissions.test.js index 74347d0f9..66d73b182 100644 --- a/ui/app/selectors/tests/permissions.test.js +++ b/ui/app/selectors/tests/permissions.test.js @@ -17,7 +17,7 @@ describe('selectors', function () { 'icon': 'https://peepeth.com/favicon-32x32.png', 'name': 'Peepeth', }, - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'icon': 'https://remix.ethereum.org/icon.png', 'name': 'Remix - Ethereum IDE', }, @@ -45,7 +45,7 @@ describe('selectors', function () { }, ], }, - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'permissions': [ { '@context': [ @@ -62,7 +62,7 @@ describe('selectors', function () { ], 'date': 1585685128948, 'id': '6b9615cc-64e4-4317-afab-3c4f8ee0244a', - 'invoker': 'remix.ethereum.org', + 'invoker': 'https://remix.ethereum.org', 'parentCapability': 'eth_accounts', }, ], @@ -80,7 +80,7 @@ describe('selectors', function () { extensionId, name: 'Remix - Ethereum IDE', icon: 'https://remix.ethereum.org/icon.png', - key: 'remix.ethereum.org', + key: 'https://remix.ethereum.org', }]) }) @@ -93,7 +93,7 @@ describe('selectors', function () { 'icon': 'https://peepeth.com/favicon-32x32.png', 'name': 'Peepeth', }, - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'icon': 'https://remix.ethereum.org/icon.png', 'name': 'Remix - Ethereum IDE', }, @@ -121,7 +121,7 @@ describe('selectors', function () { }, ], }, - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'permissions': [ { '@context': [ @@ -139,7 +139,7 @@ describe('selectors', function () { ], 'date': 1585685128948, 'id': '6b9615cc-64e4-4317-afab-3c4f8ee0244a', - 'invoker': 'remix.ethereum.org', + 'invoker': 'https://remix.ethereum.org', 'parentCapability': 'eth_accounts', }, ], @@ -152,7 +152,7 @@ describe('selectors', function () { extensionId, name: 'Remix - Ethereum IDE', icon: 'https://remix.ethereum.org/icon.png', - key: 'remix.ethereum.org', + key: 'https://remix.ethereum.org', }]) }) }) @@ -161,7 +161,7 @@ describe('selectors', function () { const mockState = { activeTab: { 'title': 'Eth Sign Tests', - 'origin': 'remix.ethereum.org', + 'origin': 'https://remix.ethereum.org', 'protocol': 'https:', 'url': 'https://remix.ethereum.org/', }, @@ -185,7 +185,7 @@ describe('selectors', function () { }, cachedBalances: {}, domains: { - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'permissions': [ { '@context': [ @@ -206,7 +206,7 @@ describe('selectors', function () { ], 'date': 1586359844177, 'id': '3aa65a8b-3bcb-4944-941b-1baa5fe0ed8b', - 'invoker': 'remix.ethereum.org', + 'invoker': 'https://remix.ethereum.org', 'parentCapability': 'eth_accounts', }, ], @@ -269,7 +269,7 @@ describe('selectors', function () { ], }], permissionsHistory: { - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'eth_accounts': { 'accounts': { '0x7250739de134d33ec7ab1ee592711e15098c9d2d': 1586359844192, @@ -323,7 +323,7 @@ describe('selectors', function () { const mockState = { activeTab: { 'title': 'Eth Sign Tests', - 'origin': 'remix.ethereum.org', + 'origin': 'https://remix.ethereum.org', 'protocol': 'https:', 'url': 'https://remix.ethereum.org/', }, @@ -343,7 +343,7 @@ describe('selectors', function () { }, }, domains: { - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'permissions': [ { '@context': [ @@ -361,7 +361,7 @@ describe('selectors', function () { ], 'date': 1586359844177, 'id': '3aa65a8b-3bcb-4944-941b-1baa5fe0ed8b', - 'invoker': 'remix.ethereum.org', + 'invoker': 'https://remix.ethereum.org', 'parentCapability': 'eth_accounts', }, ], @@ -412,13 +412,13 @@ describe('selectors', function () { }, }, domainMetadata: { - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'icon': 'https://remix.ethereum.org/icon.png', 'name': 'Remix - Ethereum IDE', }, }, permissionsHistory: { - 'remix.ethereum.org': { + 'https://remix.ethereum.org': { 'eth_accounts': { 'accounts': { '0x7250739de134d33ec7ab1ee592711e15098c9d2d': 1586359844192,