Implement Electron security guidelines (#129)

* Remove nodeintegration

* Remove parity-utils proxy

* Add CSP to renderer

* Add handling chromium permissiosn

* Remove unsafe-eval csp

* Add context isolation for preload

* Fix small bug

* Verify contextIsolation

* Remove comments

* Add some comments

* Add comment

* Fix bugs

* Re-allow eval() for builtins

electron/index.js

@@ -74,6 +74,41 @@ function createWindow () {
     callback({ requestHeaders: details.requestHeaders });
   });
 
+  // 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) => {
+      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.
+        return callback(false);
+      }
+
+      // All others loaded on file:// (shell, builtin, local) can have those
+      // permissions.
+      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) => {
+    // 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 !== encodeURI(url.format({
+      pathname: path.join(__dirname, 'preload.js'),
+      protocol: 'file:',
+      slashes: true
+    })) && !cli.uiDev) {
+      throw new Error(`Unknown preload.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`);
+    }
+
+    // Disable Node.js integration
+    webPreferences.nodeIntegration = false;
+    webPreferences.contextIsolation = true;
+  });
+
   mainWindow.on('closed', () => {
     mainWindow = null;
   });

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",

src/Dapp/dapp.css

@@ -23,7 +23,6 @@
   flex-grow: 1;
   margin: 0;
   padding: 0;
-  opacity: 0;
   width: 100%;
   z-index: 1;
 }

src/Dapp/dapp.js

@@ -93,17 +93,16 @@ 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. 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],
+        source: event.target
+      });
+    });
   };
 
   loadApp (id) {
@@ -125,7 +124,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,16 +134,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 <webview
       className={ styles.frame }
       id='dappFrame'
-      nodeintegration='false'
       preload={ preload }
       ref={ this.handleWebview }
       src={ `${src}${hash}` }
+      webpreferences='contextIsolation'
            />;
   }
 
@@ -211,10 +210,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;
-  }
 }

src/index.parity.ejs

@@ -1,35 +1,43 @@
 <!DOCTYPE html>
 <html>
-  <head>
-    <meta charset="utf-8">
-    <meta name="viewport" content="width=device-width">
-    <meta http-equiv="X-UA-Compatible" content="IE=edge">
-    <title><%= htmlWebpackPlugin.options.title %></title>
-    <style>
-      html {
-        background: white;
-        background-repeat: round;
-      }
 
-      html, body, #container {
-        width: 100%;
-        height: 100%;
-        margin: 0;
-        padding: 0;
-        font-family: Sans-Serif;
-      }
+<head>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta http-equiv="Content-Security-Policy" content="<%= htmlWebpackPlugin.options.csp %>">
+  <title>
+    <%= htmlWebpackPlugin.options.title %>
+  </title>
+  <style>
+    html {
+      background: white;
+      background-repeat: round;
+    }
 
-      .loading {
-        text-align: center;
-        padding-top: 5em;
-        font-size: 2em;
-        color: #999;
-      }
-    </style>
-  </head>
-  <body>
-    <div id="container">
-      <div class="loading">Loading</div>
-    </div>
-  </body>
-</html>
+    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;
+    }
+  </style>
+</head>
+
+<body>
+  <div id="container">
+    <div class="loading">Loading</div>
+  </div>
+</body>
+
+</html>

src/inject.js

@@ -15,11 +15,8 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 import Api from '@parity/api';
-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...
@@ -43,9 +40,9 @@ function getAppId () {
 function initProvider () {
   const appId = getAppId();
 
-  const ethereum = isElectron()
-    ? new Api.Provider.Ipc(appId)
-    : new Api.Provider.PostMessage(appId);
+  // 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}`);
 
@@ -86,4 +83,19 @@ 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
+  //
+  // 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().`);
+  // };
 }

src/preload.js

@@ -0,0 +1,38 @@
+// 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 <http://www.gnu.org/licenses/>.
+
+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 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 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 });
+});
+
+// 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);
+});

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 <webview> which are objects
+  "object-src 'self';",
+  // Allow scripts
+  "script-src 'self';"
+];
+
+module.exports = {
+  rendererCsp
+};

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');
@@ -52,7 +53,7 @@ module.exports = {
   cache: !isProd,
   devtool: isProd
     ? false
-    : isEmbed ? '#source-map' : '#eval',
+    : '#source-map',
   context: path.join(__dirname, '../src'),
   entry,
   output: {
@@ -173,6 +174,7 @@ module.exports = {
         plugins,
 
         new HtmlWebpackPlugin({
+          csp: rendererCsp.join(' '),
           title: 'Parity UI',
           filename: 'index.html',
           template: './index.parity.ejs',

webpack/inject.js

@@ -52,25 +52,25 @@ module.exports = {
       {
         test: /\.js$/,
         exclude: /node_modules/,
-        use: [ {
+        use: [{
           loader: 'happypack/loader',
           options: {
             id: 'babel'
           }
-        } ]
+        }]
       },
       {
         test: /\.json$/,
         use: ['json-loader']
       },
       {
         test: /\.html$/,
-        use: [ {
+        use: [{
           loader: 'file-loader',
           options: {
             name: '[name].[ext]'
           }
-        } ]
+        }]
       }
     ]
   },