Merge pull request #8956 from electron/expose-crash-reporter-to-sandbox

Expose crash reporter to sandbox

atom/renderer/atom_renderer_client.cc

@@ -115,7 +115,7 @@ class AtomRenderFrameObserver : public content::RenderFrameObserver {
     // an argument.
     std::string bundle(node::isolated_bundle_data,
         node::isolated_bundle_data + sizeof(node::isolated_bundle_data));
-    std::string wrapper = "(function (binding) {\n" + bundle + "\n})";
+    std::string wrapper = "(function (binding, require) {\n" + bundle + "\n})";
     auto script = v8::Script::Compile(
         mate::ConvertToV8(isolate, wrapper)->ToString());
     auto func = v8::Handle<v8::Function>::Cast(

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -9,6 +9,7 @@
 #include "atom_natives.h"  // NOLINT: This file is generated with js2c
 
 #include "atom/common/api/api_messages.h"
+#include "atom/common/api/atom_bindings.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "atom/common/native_mate_converters/v8_value_converter.h"
 #include "atom/common/native_mate_converters/value_converter.h"
@@ -85,6 +86,7 @@ void InitializeBindings(v8::Local<v8::Object> binding,
   auto isolate = context->GetIsolate();
   mate::Dictionary b(isolate, binding);
   b.SetMethod("get", GetBinding);
+  b.SetMethod("crash", AtomBindings::Crash);
 }
 
 class AtomSandboxedRenderFrameObserver : public content::RenderFrameObserver {
@@ -204,7 +206,7 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
   std::string preload_bundle_native(node::preload_bundle_data,
       node::preload_bundle_data + sizeof(node::preload_bundle_data));
   std::stringstream ss;
-  ss << "(function(binding, preloadPath) {\n";
+  ss << "(function(binding, preloadPath, require) {\n";
   ss << preload_bundle_native << "\n";
   ss << "})";
   std::string preload_wrapper = ss.str();

electron.gyp

@@ -441,7 +441,13 @@
         'sandbox_args': [
           './lib/sandboxed_renderer/init.js',
           '-r',
-          './lib/sandboxed_renderer/api/exports/electron.js:electron'
+          './lib/sandboxed_renderer/api/exports/electron.js:electron',
+          '-r',
+          './lib/sandboxed_renderer/api/exports/fs.js:fs',
+          '-r',
+          './lib/sandboxed_renderer/api/exports/os.js:os',
+          '-r',
+          './lib/sandboxed_renderer/api/exports/child_process.js:child_process'
         ],
         'isolated_args': [
           'lib/isolated_renderer/init.js',

lib/sandboxed_renderer/api/exports/child_process.js

@@ -0,0 +1 @@
+module.exports = require('electron').remote.require('child_process')

lib/sandboxed_renderer/api/exports/electron.js

@@ -11,6 +11,12 @@ Object.defineProperties(exports, {
       return require('../../../renderer/api/remote')
     }
   },
+  crashReporter: {
+    enumerable: true,
+    get: function () {
+      return require('../../../common/api/crash-reporter')
+    }
+  },
   CallbacksRegistry: {
     get: function () {
       return require('../../../common/api/callbacks-registry')

lib/sandboxed_renderer/api/exports/fs.js

@@ -0,0 +1 @@
+module.exports = require('electron').remote.require('fs')

lib/sandboxed_renderer/api/exports/os.js

@@ -0,0 +1 @@
+module.exports = require('electron').remote.require('os')

lib/sandboxed_renderer/init.js

@@ -21,23 +21,23 @@ for (let prop of Object.keys(events.EventEmitter.prototype)) {
 Object.setPrototypeOf(process, events.EventEmitter.prototype)
 
 const electron = require('electron')
+const fs = require('fs')
 const preloadModules = new Map([
-  ['electron', electron]
+  ['child_process', require('child_process')],
+  ['electron', electron],
+  ['fs', fs],
+  ['os', require('os')],
+  ['url', require('url')],
+  ['timers', require('timers')]
 ])
 
-const extraModules = [
+const preloadSrc = fs.readFileSync(preloadPath).toString()
-  'fs'
-]
-for (let extraModule of extraModules) {
-  preloadModules.set(extraModule, electron.remote.require(extraModule))
-}
-
-// Fetch the preload script using the "fs" module proxy.
-let preloadSrc = preloadModules.get('fs').readFileSync(preloadPath).toString()
 
 // Pass different process object to the preload script(which should not have
 // access to things like `process.atomBinding`).
 const preloadProcess = new events.EventEmitter()
+preloadProcess.platform = electron.remote.process.platform
+preloadProcess.crash = () => binding.crash()
 process.on('exit', () => preloadProcess.emit('exit'))
 
 // This is the `require` function that will be visible to the preload script
@@ -67,12 +67,13 @@ function preloadRequire (module) {
 // and any `require('electron')` calls in `preload.js` will work as expected
 // since browserify won't try to include `electron` in the bundle, falling back
 // to the `preloadRequire` function above.
-let preloadWrapperSrc = `(function(require, process, Buffer, global) {
+const preloadWrapperSrc = `(function(require, process, Buffer, global, setImmediate) {
 ${preloadSrc}
 })`
 
 // eval in window scope:
 // http://www.ecma-international.org/ecma-262/5.1/#sec-10.4.2
 const geval = eval
-let preloadFn = geval(preloadWrapperSrc)
+const preloadFn = geval(preloadWrapperSrc)
-preloadFn(preloadRequire, preloadProcess, Buffer, global)
+const {setImmediate} = require('timers')
+preloadFn(preloadRequire, preloadProcess, Buffer, global, setImmediate)

spec/api-crash-reporter-spec.js

@@ -11,85 +11,96 @@ const {remote} = require('electron')
 const {app, BrowserWindow, crashReporter} = remote.require('electron')
 
 describe('crashReporter module', function () {
-  var fixtures = path.resolve(__dirname, 'fixtures')
-  var w = null
-  var originalTempDirectory = null
-  var tempDirectory = null
-
-  beforeEach(function () {
-    w = new BrowserWindow({
-      show: false
-    })
-    tempDirectory = temp.mkdirSync('electronCrashReporterSpec-')
-    originalTempDirectory = app.getPath('temp')
-    app.setPath('temp', tempDirectory)
-  })
-
-  afterEach(function () {
-    app.setPath('temp', originalTempDirectory)
-    return closeWindow(w).then(function () { w = null })
-  })
-
   if (process.mas) {
     return
   }
+  var fixtures = path.resolve(__dirname, 'fixtures')
+  const generateSpecs = (description, browserWindowOpts) => {
+    describe(description, function () {
+      var w = null
+      var originalTempDirectory = null
+      var tempDirectory = null
 
-  it('should send minidump when renderer crashes', function (done) {
+      beforeEach(function () {
-    if (process.env.APPVEYOR === 'True') return done()
+        w = new BrowserWindow(Object.assign({
-    if (process.env.TRAVIS === 'true') return done()
+          show: false
+        }, browserWindowOpts))
+        tempDirectory = temp.mkdirSync('electronCrashReporterSpec-')
+        originalTempDirectory = app.getPath('temp')
+        app.setPath('temp', tempDirectory)
+      })
 
-    this.timeout(120000)
+      afterEach(function () {
+        app.setPath('temp', originalTempDirectory)
+        return closeWindow(w).then(function () { w = null })
+      })
 
-    startServer({
+      it('should send minidump when renderer crashes', function (done) {
-      callback (port) {
+        if (process.env.APPVEYOR === 'True') return done()
-        const crashUrl = url.format({
+        if (process.env.TRAVIS === 'true') return done()
-          protocol: 'file',
+
-          pathname: path.join(fixtures, 'api', 'crash.html'),
+        this.timeout(120000)
-          search: '?port=' + port
+
+        startServer({
+          callback (port) {
+            const crashUrl = url.format({
+              protocol: 'file',
+              pathname: path.join(fixtures, 'api', 'crash.html'),
+              search: '?port=' + port
+            })
+            w.loadURL(crashUrl)
+          },
+          processType: 'renderer',
+          done: done
         })
-        w.loadURL(crashUrl)
+      })
-      },
-      processType: 'renderer',
-      done: done
-    })
-  })
 
-  it('should send minidump when node processes crash', function (done) {
+      it('should send minidump when node processes crash', function (done) {
-    if (process.env.APPVEYOR === 'True') return done()
+        if (process.env.APPVEYOR === 'True') return done()
-    if (process.env.TRAVIS === 'true') return done()
+        if (process.env.TRAVIS === 'true') return done()
 
-    this.timeout(120000)
+        this.timeout(120000)
 
-    startServer({
+        startServer({
-      callback (port) {
+          callback (port) {
-        const crashesDir = path.join(app.getPath('temp'), `${app.getName()} Crashes`)
+            const crashesDir = path.join(app.getPath('temp'), `${app.getName()} Crashes`)
-        const version = app.getVersion()
+            const version = app.getVersion()
-        const crashPath = path.join(fixtures, 'module', 'crash.js')
+            const crashPath = path.join(fixtures, 'module', 'crash.js')
-        childProcess.fork(crashPath, [port, version, crashesDir], {silent: true})
+            childProcess.fork(crashPath, [port, version, crashesDir], {silent: true})
-      },
+          },
-      processType: 'browser',
+          processType: 'browser',
-      done: done
+          done: done
-    })
-  })
-
-  it('should send minidump with updated extra parameters', function (done) {
-    if (process.env.APPVEYOR === 'True') return done()
-    if (process.env.TRAVIS === 'true') return done()
-
-    this.timeout(10000)
-
-    startServer({
-      callback (port) {
-        const crashUrl = url.format({
-          protocol: 'file',
-          pathname: path.join(fixtures, 'api', 'crash-restart.html'),
-          search: '?port=' + port
         })
-        w.loadURL(crashUrl)
+      })
-      },
+
-      processType: 'renderer',
+      it('should send minidump with updated extra parameters', function (done) {
-      done: done
+        if (process.env.APPVEYOR === 'True') return done()
+        if (process.env.TRAVIS === 'true') return done()
+
+        this.timeout(10000)
+
+        startServer({
+          callback (port) {
+            const crashUrl = url.format({
+              protocol: 'file',
+              pathname: path.join(fixtures, 'api', 'crash-restart.html'),
+              search: '?port=' + port
+            })
+            w.loadURL(crashUrl)
+          },
+          processType: 'renderer',
+          done: done
+        })
+      })
     })
+  }
+
+  generateSpecs('without sandbox', {})
+  generateSpecs('with sandbox ', {
+    webPreferences: {
+      sandbox: true,
+      preload: path.join(fixtures, 'module', 'preload-sandbox.js')
+    }
   })
 
   describe('.start(options)', function () {

spec/fixtures/module/preload-sandbox.js

@@ -1,6 +1,8 @@
 (function () {
+  const {setImmediate} = require('timers')
   const {ipcRenderer} = require('electron')
   window.ipcRenderer = ipcRenderer
+  window.setImmediate = setImmediate
   if (location.protocol === 'file:') {
     window.test = 'preload'
     window.require = require