From 5229d8f57c15a81faec5740f2506a2ed181e7194 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 31 May 2018 13:56:00 +0300 Subject: [PATCH 01/13] Remove nodeintegration --- electron/index.js | 18 ++++++++++++++++++ src/Dapp/dapp.js | 2 +- src/inject.js | 31 +++++++++++++++++++++++++++++++ webpack/inject.js | 10 ++++++---- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/electron/index.js b/electron/index.js index 81af0e663..3e724f14f 100644 --- a/electron/index.js +++ b/electron/index.js @@ -74,6 +74,24 @@ function createWindow () { callback({ requestHeaders: details.requestHeaders }); }); + // Verify WebView Options Before Creation + // https://electronjs.org/docs/tutorial/security#12-verify-webview-options-before-creation + mainWindow.webContents.on('will-attach-webview', (event, webPreferences, params) => { + // Strip away preload scripts if unused + delete webPreferences.preload; + // Verify the location of our prelaod script is legitimate (unless uiDev has been passed) + if (webPreferences.preloadURL !== url.format({ + pathname: path.join(__dirname, 'inject.js'), + protocol: 'file:', + slashes: true + }) && !cli.uiDev) { + throw new Error(`Unknown preload inject.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); + } + + // Disable Node.js integration + webPreferences.nodeIntegration = false; + }); + mainWindow.on('closed', () => { mainWindow = null; }); diff --git a/src/Dapp/dapp.js b/src/Dapp/dapp.js index e6a23011e..138ff01ad 100644 --- a/src/Dapp/dapp.js +++ b/src/Dapp/dapp.js @@ -139,10 +139,10 @@ export default class Dapp extends Component { 'inject.js' )}`; + // https://electronjs.org/docs/tutorial/security#3-enable-context-isolation-for-remote-content return . +// This needs to be run before importing @parity/api, as it will populate +// window.parity.electron, which the IPC Provider will use. +initIpc(); + import Api from '@parity/api'; +import { ipcRenderer } from 'electron'; import isElectron from 'is-electron'; import qs from 'query-string'; @@ -40,6 +45,26 @@ function getAppId () { console.error('Could not find appId'); } +function initIpc () { + // Ipc object to be injected into window + if (typeof window !== 'undefined') { + if (!window.parity) { + window.parity = {}; + } + window.parity.electron = { + onReceiveMessage (fn) { + ipcRenderer.on('PARITY_SHELL_IPC_CHANNEL', fn); + }, + onReceivePing (fn) { + ipcRenderer.once('ping', fn); + }, + sendToHost (data) { + ipcRenderer.sendToHost('parity', data); + } + }; + } +} + function initProvider () { const appId = getAppId(); @@ -86,4 +111,10 @@ if (typeof window !== 'undefined' && !window.isParity) { initParity(ethereum); console.warn('Deprecation: Dapps should only used the exposed EthereumProvider on `window.ethereum`, the use of `window.parity` and `window.web3` will be removed in future versions of this injector'); + + // Disable eval() for dapps + // https://electronjs.org/docs/tutorial/security#7-override-and-disable-eval + window.eval = global.eval = function () { // eslint-disable-line + throw new Error(`Sorry, this app does not support window.eval().`); + }; } diff --git a/webpack/inject.js b/webpack/inject.js index 8d55ea576..9833d9a97 100644 --- a/webpack/inject.js +++ b/webpack/inject.js @@ -37,6 +37,8 @@ module.exports = { libraryTarget: 'umd' }, + target: 'electron-renderer', + resolve: { alias: {} }, @@ -52,12 +54,12 @@ module.exports = { { test: /\.js$/, exclude: /node_modules/, - use: [ { + use: [{ loader: 'happypack/loader', options: { id: 'babel' } - } ] + }] }, { test: /\.json$/, @@ -65,12 +67,12 @@ module.exports = { }, { test: /\.html$/, - use: [ { + use: [{ loader: 'file-loader', options: { name: '[name].[ext]' } - } ] + }] } ] }, From 0d79fde64b13f74ee4d7e2edadb070cd7f0bcbea Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 31 May 2018 14:00:33 +0300 Subject: [PATCH 02/13] Remove parity-utils proxy --- webpack/shared.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/webpack/shared.js b/webpack/shared.js index 11b2a38e7..d7857db54 100644 --- a/webpack/shared.js +++ b/webpack/shared.js @@ -90,14 +90,6 @@ function addProxies (app) { } })); - app.use('/parity-utils', proxy({ - target: 'http://127.0.0.1:3000', - changeOrigin: true, - pathRewrite: { - '^/parity-utils': '' - } - })); - app.use('/rpc', proxy({ target: 'http://127.0.0.1:8545', changeOrigin: true From 4c2a4b5a4bd2413e78e58fc571bace9bbb130dc3 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 31 May 2018 16:47:54 +0200 Subject: [PATCH 03/13] Add CSP to renderer --- src/index.parity.ejs | 70 ++++++++++++++++++++++++-------------------- src/util/csp.js | 41 ++++++++++++++++++++++++++ webpack/app.js | 2 ++ 3 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 src/util/csp.js diff --git a/src/index.parity.ejs b/src/index.parity.ejs index 7009ce0ee..f774c825a 100644 --- a/src/index.parity.ejs +++ b/src/index.parity.ejs @@ -1,35 +1,43 @@ - - - - - <%= htmlWebpackPlugin.options.title %> - - - -
-
Loading
-
- - + html, + body, + #container { + width: 100%; + height: 100%; + margin: 0; + padding: 0; + font-family: Sans-Serif; + } + + .loading { + text-align: center; + padding-top: 5em; + font-size: 2em; + color: #999; + } + + + + +
+
Loading
+
+ + + \ No newline at end of file diff --git a/src/util/csp.js b/src/util/csp.js new file mode 100644 index 000000000..e4745ba7a --- /dev/null +++ b/src/util/csp.js @@ -0,0 +1,41 @@ +const shared = [ + // Restrict everything to the same origin by default. + "default-src 'self';", + // Allow connecting to WS servers and HTTP(S) servers. + // We could be more restrictive and allow only RPC server URL. + 'connect-src http: https: ws: wss:;', + // Allow framing any content from HTTP(S). + // Again we could only allow embedding from RPC server URL. + // (deprecated) + "frame-src 'none';", + // Allow framing and web workers from HTTP(S). + "child-src 'self' http: https:;", + // We allow data: blob: and HTTP(s) images. + // We could get rid of wildcarding HTTP and only allow RPC server URL. + // (http required for local dapps icons) + "img-src 'self' 'unsafe-inline' file: data: blob: http: https:;", + // Allow style from data: blob: and HTTPS. + "style-src 'self' 'unsafe-inline' data: blob: https:;", + // Allow fonts from data: and HTTPS. + "font-src 'self' data: https:;", + // Same restrictions as script-src with additional + // blob: that is required for camera access (worker) + "worker-src 'self' https: blob:;", + // Disallow submitting forms from any dapps + "form-action 'none';", + // Never allow mixed content + 'block-all-mixed-content;' +]; + +// These are the CSP for the renderer process (aka the shell) +const rendererCsp = [ + ...shared, + // Allow which are objects + "object-src 'self';", + // Allow scripts, unfortunately needs unsafe-eval TODO + "script-src 'self' 'unsafe-eval';" +]; + +module.exports = { + rendererCsp +}; diff --git a/webpack/app.js b/webpack/app.js index 3e701d650..88a9d73a5 100644 --- a/webpack/app.js +++ b/webpack/app.js @@ -26,6 +26,7 @@ const CopyWebpackPlugin = require('copy-webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); const ServiceWorkerWebpackPlugin = require('serviceworker-webpack-plugin'); +const { rendererCsp } = require('../src/util/csp'); const rulesEs6 = require('./rules/es6'); const rulesParity = require('./rules/parity'); const Shared = require('./shared'); @@ -173,6 +174,7 @@ module.exports = { plugins, new HtmlWebpackPlugin({ + csp: rendererCsp.join(' '), title: 'Parity UI', filename: 'index.html', template: './index.parity.ejs', From 75bd0aa5629a5b5ca53309b2aabb2b7a7260e3b4 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 31 May 2018 18:04:55 +0200 Subject: [PATCH 04/13] Add handling chromium permissiosn --- electron/index.js | 12 ++++++++++++ src/Dapp/dapp.js | 1 - 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/electron/index.js b/electron/index.js index 3e724f14f..74bc00919 100644 --- a/electron/index.js +++ b/electron/index.js @@ -74,6 +74,18 @@ function createWindow () { callback({ requestHeaders: details.requestHeaders }); }); + // Do not accept all kind of web permissions + // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content + session.defaultSession + .setPermissionRequestHandler((webContents, permission, callback) => { + if (!url.startsWith('file:')) { + // Denies the permissions request for all non-file:// + return callback(false); + } + + return callback(true); + }); + // Verify WebView Options Before Creation // https://electronjs.org/docs/tutorial/security#12-verify-webview-options-before-creation mainWindow.webContents.on('will-attach-webview', (event, webPreferences, params) => { diff --git a/src/Dapp/dapp.js b/src/Dapp/dapp.js index 138ff01ad..2b8f56a3f 100644 --- a/src/Dapp/dapp.js +++ b/src/Dapp/dapp.js @@ -139,7 +139,6 @@ export default class Dapp extends Component { 'inject.js' )}`; - // https://electronjs.org/docs/tutorial/security#3-enable-context-isolation-for-remote-content return Date: Fri, 1 Jun 2018 15:27:24 +0200 Subject: [PATCH 05/13] Remove unsafe-eval csp --- src/util/csp.js | 2 +- webpack/app.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/csp.js b/src/util/csp.js index e4745ba7a..f01de1030 100644 --- a/src/util/csp.js +++ b/src/util/csp.js @@ -33,7 +33,7 @@ const rendererCsp = [ // Allow which are objects "object-src 'self';", // Allow scripts, unfortunately needs unsafe-eval TODO - "script-src 'self' 'unsafe-eval';" + "script-src 'self';" ]; module.exports = { diff --git a/webpack/app.js b/webpack/app.js index 88a9d73a5..3b51e6f4c 100644 --- a/webpack/app.js +++ b/webpack/app.js @@ -53,7 +53,7 @@ module.exports = { cache: !isProd, devtool: isProd ? false - : isEmbed ? '#source-map' : '#eval', + : '#source-map', context: path.join(__dirname, '../src'), entry, output: { From 2b00ab49bccc49826c21456a60620c45f1efea64 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Fri, 1 Jun 2018 20:10:48 +0200 Subject: [PATCH 06/13] Add context isolation for preload --- package.json | 4 +-- src/Dapp/dapp.css | 1 - src/Dapp/dapp.js | 27 ++++++++------------ src/inject.js | 29 +--------------------- src/preload.js | 36 +++++++++++++++++++++++++++ src/util/csp.js | 2 +- webpack/inject.js | 2 -- webpack/preload.js | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 111 insertions(+), 51 deletions(-) create mode 100644 src/preload.js create mode 100644 webpack/preload.js diff --git a/package.json b/package.json index defc8b637..eac012466 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "build": "npm run build:inject && npm run build:app && npm run build:electron && npm run build:embed", "build:app": "webpack --config webpack/app", "build:electron": "webpack --config webpack/electron", - "build:inject": "webpack --config webpack/inject", + "build:inject": "webpack --config webpack/inject && webpack --config webpack/preload", "build:embed": "cross-env EMBED=1 node webpack/embed", "build:i18n": "npm run clean && npm run build && babel-node ./scripts/build-i18n.js", "ci:build": "cross-env NODE_ENV=production npm run build", @@ -195,4 +195,4 @@ "store": "1.3.20", "util.promisify": "1.0.0" } -} +} \ No newline at end of file diff --git a/src/Dapp/dapp.css b/src/Dapp/dapp.css index a458aa7a4..3cd0684d4 100644 --- a/src/Dapp/dapp.css +++ b/src/Dapp/dapp.css @@ -23,7 +23,6 @@ flex-grow: 1; margin: 0; padding: 0; - opacity: 0; width: 100%; z-index: 1; } diff --git a/src/Dapp/dapp.js b/src/Dapp/dapp.js index 2b8f56a3f..c1e49abed 100644 --- a/src/Dapp/dapp.js +++ b/src/Dapp/dapp.js @@ -93,17 +93,15 @@ export default class Dapp extends Component { // Reput eventListeners when webview has finished loading dapp webview.addEventListener('did-finish-load', () => { this.setState({ loading: false }); - // Listen to IPC messages from this webview - webview.addEventListener('ipc-message', event => - this.requestsStore.receiveMessage({ - ...event.args[0], - source: event.target - })); - // Send ping message to tell dapp we're ready to listen to its ipc messages - webview.send('ping'); }); - this.onDappLoad(); + // Listen to IPC messages from this webview + webview.addEventListener('ipc-message', event => { + this.requestsStore.receiveMessage({ + ...event.args[0], + source: event.target + }); + }); }; loadApp (id) { @@ -125,7 +123,6 @@ export default class Dapp extends Component { frameBorder={ 0 } id='dappFrame' name={ name } - onLoad={ this.onDappLoad } ref={ this.handleIframe } sandbox='allow-forms allow-popups allow-same-origin allow-scripts allow-top-navigation' scrolling='auto' @@ -136,15 +133,17 @@ export default class Dapp extends Component { renderWebview = (src, hash) => { const preload = `file://${path.join( getBuildPath(), - 'inject.js' + 'preload.js' )}`; + // https://electronjs.org/docs/tutorial/security#3-enable-context-isolation-for-remote-content return ; } @@ -210,10 +209,4 @@ export default class Dapp extends Component { ? this.renderWebview(src, hash) : this.renderIframe(src, hash); } - - onDappLoad = () => { - const frame = document.getElementById('dappFrame'); - - frame.style.opacity = 1; - } } diff --git a/src/inject.js b/src/inject.js index 114c62fa2..844dfeeff 100644 --- a/src/inject.js +++ b/src/inject.js @@ -16,15 +16,10 @@ // This needs to be run before importing @parity/api, as it will populate // window.parity.electron, which the IPC Provider will use. -initIpc(); import Api from '@parity/api'; -import { ipcRenderer } from 'electron'; -import isElectron from 'is-electron'; import qs from 'query-string'; -console.log('This inject.js has been injected by the shell.'); - function getAppId () { // Dapps built into the shell; URL: file://path-to-shell/.build/dapps/0x0587.../index.html // Dapps installed from the registry and served by Parity; URL: http://127.0.0.1:8545/ff19... @@ -45,32 +40,10 @@ function getAppId () { console.error('Could not find appId'); } -function initIpc () { - // Ipc object to be injected into window - if (typeof window !== 'undefined') { - if (!window.parity) { - window.parity = {}; - } - window.parity.electron = { - onReceiveMessage (fn) { - ipcRenderer.on('PARITY_SHELL_IPC_CHANNEL', fn); - }, - onReceivePing (fn) { - ipcRenderer.once('ping', fn); - }, - sendToHost (data) { - ipcRenderer.sendToHost('parity', data); - } - }; - } -} - function initProvider () { const appId = getAppId(); - const ethereum = isElectron() - ? new Api.Provider.Ipc(appId) - : new Api.Provider.PostMessage(appId); + const ethereum = new Api.Provider.PostMessage(appId); console.log(`Requesting API communications token for ${appId}`); diff --git a/src/preload.js b/src/preload.js new file mode 100644 index 000000000..646092a68 --- /dev/null +++ b/src/preload.js @@ -0,0 +1,36 @@ +// Copyright 2015-2017 Parity Technologies (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +import fs from 'fs'; +import { ipcRenderer, remote, webFrame } from 'electron'; +import path from 'path'; + +// The following two lines is just a proxy between the webview and the renderer process. +// If we receive an IPC message, we send it to the webview as postMessage +ipcRenderer.on('PARITY_SHELL_IPC_CHANNEL', (_, data) => window.postMessage(data, '*')); +// If we receive a postMessage, we transfer it to the renderer as an IPC message +window.addEventListener('message', (event) => { + ipcRenderer.sendToHost('PARITY_SHELL_IPC_CHANNEL', { data: event.data }); +}); + +// Load inject.js as a string, and inject it into the webview with executeJavaScript +fs.readFile(path.join(remote.getGlobal('dirName'), '..', '.build', 'inject.js'), 'utf8', (err, injectFile) => { + if (err) { + console.error(err); + return; + } + webFrame.executeJavaScript(injectFile); +}); diff --git a/src/util/csp.js b/src/util/csp.js index f01de1030..c85f962d2 100644 --- a/src/util/csp.js +++ b/src/util/csp.js @@ -32,7 +32,7 @@ const rendererCsp = [ ...shared, // Allow which are objects "object-src 'self';", - // Allow scripts, unfortunately needs unsafe-eval TODO + // Allow scripts "script-src 'self';" ]; diff --git a/webpack/inject.js b/webpack/inject.js index 9833d9a97..817fcfe81 100644 --- a/webpack/inject.js +++ b/webpack/inject.js @@ -37,8 +37,6 @@ module.exports = { libraryTarget: 'umd' }, - target: 'electron-renderer', - resolve: { alias: {} }, diff --git a/webpack/preload.js b/webpack/preload.js new file mode 100644 index 000000000..dbb2faf02 --- /dev/null +++ b/webpack/preload.js @@ -0,0 +1,61 @@ +// Copyright 2015-2017 Parity Technologies (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +const path = require('path'); + +const rulesEs6 = require('./rules/es6'); +const rulesParity = require('./rules/parity'); +const Shared = require('./shared'); + +const DEST = process.env.BUILD_DEST || '.build'; + +module.exports = { + context: path.join(__dirname, '../src'), + devtool: false, + entry: './preload.js', + output: { + path: path.join(__dirname, '../', DEST), + filename: 'preload.js' + }, + + target: 'electron-renderer', + + resolve: { + alias: {} + }, + + node: { + fs: 'empty' + }, + + module: { + rules: [ + rulesParity, + rulesEs6, + { + test: /\.js$/, + exclude: /node_modules/, + use: [{ + loader: 'happypack/loader', + options: { + id: 'babel' + } + }] + } + ] + }, + plugins: Shared.getPlugins() +}; From 0566a13525a3e93cc1de5378e98066bd6bc389b5 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Fri, 1 Jun 2018 20:14:23 +0200 Subject: [PATCH 07/13] Fix small bug --- electron/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/electron/index.js b/electron/index.js index 74bc00919..d5bf0a9d6 100644 --- a/electron/index.js +++ b/electron/index.js @@ -93,11 +93,11 @@ function createWindow () { delete webPreferences.preload; // Verify the location of our prelaod script is legitimate (unless uiDev has been passed) if (webPreferences.preloadURL !== url.format({ - pathname: path.join(__dirname, 'inject.js'), + pathname: path.join(__dirname, 'preload.js'), protocol: 'file:', slashes: true }) && !cli.uiDev) { - throw new Error(`Unknown preload inject.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); + throw new Error(`Unknown preload preload.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); } // Disable Node.js integration From 7083b2274e343ac95edf3d99522d6cf3497e731f Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Fri, 1 Jun 2018 20:59:19 +0200 Subject: [PATCH 08/13] Verify contextIsolation --- electron/index.js | 3 ++- package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/electron/index.js b/electron/index.js index d5bf0a9d6..2defebeb2 100644 --- a/electron/index.js +++ b/electron/index.js @@ -89,7 +89,7 @@ function createWindow () { // Verify WebView Options Before Creation // https://electronjs.org/docs/tutorial/security#12-verify-webview-options-before-creation mainWindow.webContents.on('will-attach-webview', (event, webPreferences, params) => { - // Strip away preload scripts if unused + // Strip away inline preload scripts, ours is at preloadURL delete webPreferences.preload; // Verify the location of our prelaod script is legitimate (unless uiDev has been passed) if (webPreferences.preloadURL !== url.format({ @@ -102,6 +102,7 @@ function createWindow () { // Disable Node.js integration webPreferences.nodeIntegration = false; + webPreferences.contextIsolation = true; }); mainWindow.on('closed', () => { diff --git a/package.json b/package.json index eac012466..558f12f0a 100644 --- a/package.json +++ b/package.json @@ -195,4 +195,4 @@ "store": "1.3.20", "util.promisify": "1.0.0" } -} \ No newline at end of file +} From cadc67b893fb179a97f97e31c9d292f48fdd4805 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Fri, 1 Jun 2018 21:02:59 +0200 Subject: [PATCH 09/13] Remove comments --- src/inject.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/inject.js b/src/inject.js index 844dfeeff..682c5cf1b 100644 --- a/src/inject.js +++ b/src/inject.js @@ -14,9 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -// This needs to be run before importing @parity/api, as it will populate -// window.parity.electron, which the IPC Provider will use. - import Api from '@parity/api'; import qs from 'query-string'; From 78a82d5c1052dc7ef47c275ef372b5f4644a9505 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 4 Jun 2018 10:03:23 +0200 Subject: [PATCH 10/13] Add some comments --- electron/index.js | 6 +++++- src/Dapp/dapp.js | 3 ++- src/inject.js | 2 ++ src/preload.js | 6 ++++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/electron/index.js b/electron/index.js index 2defebeb2..280a9533b 100644 --- a/electron/index.js +++ b/electron/index.js @@ -79,10 +79,14 @@ function createWindow () { session.defaultSession .setPermissionRequestHandler((webContents, permission, callback) => { if (!url.startsWith('file:')) { - // Denies the permissions request for all non-file:// + // Denies the permissions request for all non-file://. Currently all + // network dapps are loaded on http://127.0.0.1:8545, so they won't + // have any permissions. return callback(false); } + // All others loaded on file:// (shell, builtin, local) can have those + // permissions. return callback(true); }); diff --git a/src/Dapp/dapp.js b/src/Dapp/dapp.js index c1e49abed..35ef687d0 100644 --- a/src/Dapp/dapp.js +++ b/src/Dapp/dapp.js @@ -95,7 +95,8 @@ export default class Dapp extends Component { this.setState({ loading: false }); }); - // Listen to IPC messages from this webview + // Listen to IPC messages from this webview. More particularly, to IPC + // messages coming from the preload.js injected in this webview. webview.addEventListener('ipc-message', event => { this.requestsStore.receiveMessage({ ...event.args[0], diff --git a/src/inject.js b/src/inject.js index 682c5cf1b..20d48633b 100644 --- a/src/inject.js +++ b/src/inject.js @@ -40,6 +40,8 @@ function getAppId () { function initProvider () { const appId = getAppId(); + // The dapp will use the PostMessage provider, send postMessages to + // preload.js, and preload.js will relay those messages to the shell. const ethereum = new Api.Provider.PostMessage(appId); console.log(`Requesting API communications token for ${appId}`); diff --git a/src/preload.js b/src/preload.js index 646092a68..359707fb9 100644 --- a/src/preload.js +++ b/src/preload.js @@ -19,9 +19,11 @@ import { ipcRenderer, remote, webFrame } from 'electron'; import path from 'path'; // The following two lines is just a proxy between the webview and the renderer process. -// If we receive an IPC message, we send it to the webview as postMessage +// If we receive an IPC message from the shell, we relay it to the webview as +// postMessage. ipcRenderer.on('PARITY_SHELL_IPC_CHANNEL', (_, data) => window.postMessage(data, '*')); -// If we receive a postMessage, we transfer it to the renderer as an IPC message +// If we receive a postMessage from the webview, we transfer it to the renderer +// as an IPC message. window.addEventListener('message', (event) => { ipcRenderer.sendToHost('PARITY_SHELL_IPC_CHANNEL', { data: event.data }); }); From cba501d7a07e464d188b63dc46b47dcd8cf7d2cf Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 4 Jun 2018 10:06:59 +0200 Subject: [PATCH 11/13] Add comment --- electron/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/electron/index.js b/electron/index.js index 280a9533b..38cfe7b16 100644 --- a/electron/index.js +++ b/electron/index.js @@ -74,7 +74,7 @@ function createWindow () { callback({ requestHeaders: details.requestHeaders }); }); - // Do not accept all kind of web permissions + // Do not accept all kind of web permissions (camera, location...) // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content session.defaultSession .setPermissionRequestHandler((webContents, permission, callback) => { From 7ab0f2cde7022ccc6b0de9ea8ee56da9df8f26ee Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 4 Jun 2018 13:08:45 +0200 Subject: [PATCH 12/13] Fix bugs --- electron/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/electron/index.js b/electron/index.js index 38cfe7b16..8882e7e1a 100644 --- a/electron/index.js +++ b/electron/index.js @@ -78,7 +78,7 @@ function createWindow () { // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content session.defaultSession .setPermissionRequestHandler((webContents, permission, callback) => { - if (!url.startsWith('file:')) { + if (!webContents.getURL().startsWith('file:')) { // Denies the permissions request for all non-file://. Currently all // network dapps are loaded on http://127.0.0.1:8545, so they won't // have any permissions. @@ -96,12 +96,12 @@ function createWindow () { // Strip away inline preload scripts, ours is at preloadURL delete webPreferences.preload; // Verify the location of our prelaod script is legitimate (unless uiDev has been passed) - if (webPreferences.preloadURL !== url.format({ + if (webPreferences.preloadURL !== encodeURI(url.format({ pathname: path.join(__dirname, 'preload.js'), protocol: 'file:', slashes: true - }) && !cli.uiDev) { - throw new Error(`Unknown preload preload.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); + })) && !cli.uiDev) { + throw new Error(`Unknown preload.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); } // Disable Node.js integration From e945c492bfcb44dce11bc68bba4b2fc4745893c5 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 4 Jun 2018 13:34:37 +0200 Subject: [PATCH 13/13] Re-allow eval() for builtins --- src/inject.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/inject.js b/src/inject.js index 20d48633b..54e2d5bc7 100644 --- a/src/inject.js +++ b/src/inject.js @@ -86,7 +86,16 @@ if (typeof window !== 'undefined' && !window.isParity) { // Disable eval() for dapps // https://electronjs.org/docs/tutorial/security#7-override-and-disable-eval - window.eval = global.eval = function () { // eslint-disable-line - throw new Error(`Sorry, this app does not support window.eval().`); - }; + // + // TODO Currently Web3 Console dapp needs eval(), and is the only builtin + // that needs it, so we cannot blindly disable it as per the recommendation. + // One idea is to check here in inject.js if allowJsEval is set to true, but + // this requires more work (future PR). + // For now we simply allow eval(). All builtin dapps are trusted and can use + // eval(), and all network dapps are served on 127.0.0.1:8545, which have CSP + // that disallow eval(). So security-wise it should be enough. + // + // window.eval = global.eval = function () { // eslint-disable-line + // throw new Error(`Sorry, this app does not support window.eval().`); + // }; }