Add webPreferences.disableRemote option to disable the remote module …

…(sandbox security)
electron · Aug 25, 2018 · e348511 · e348511
1 parent d241964
commit e348511
Show file tree

Hide file tree

Showing 20 changed files with 149 additions and 70 deletions.
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
@@ -355,6 +355,9 @@ WebContents::WebContents(v8::Isolate* isolate,
   // Whether to enable DevTools.
   options.Get("devTools", &enable_devtools_);
 
+  // Whether to disable remote.
+  options.Get("disableRemote", &disable_remote_);
+
   // Obtain the session.
   std::string partition;
   mate::Handle<api::Session> session;
@@ -1880,6 +1883,10 @@ v8::Local<v8::Value> WebContents::GetLastWebPreferences(
   return mate::ConvertToV8(isolate, *web_preferences->last_preference());
 }
 
+bool WebContents::IsRemoteDisabled() const {
+  return disable_remote_;
+}
+
 v8::Local<v8::Value> WebContents::GetOwnerBrowserWindow() const {
   if (owner_window())
     return BrowserWindow::From(isolate(), owner_window());
@@ -2030,6 +2037,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("_getPreloadPath", &WebContents::GetPreloadPath)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)
       .SetMethod("getLastWebPreferences", &WebContents::GetLastWebPreferences)
+      .SetMethod("_isRemoteDisabled", &WebContents::IsRemoteDisabled)
       .SetMethod("getOwnerBrowserWindow", &WebContents::GetOwnerBrowserWindow)
       .SetMethod("hasServiceWorker", &WebContents::HasServiceWorker)
       .SetMethod("unregisterServiceWorker",

diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h
@@ -237,6 +237,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
   v8::Local<v8::Value> GetWebPreferences(v8::Isolate* isolate) const;
   v8::Local<v8::Value> GetLastWebPreferences(v8::Isolate* isolate) const;
 
+  bool IsRemoteDisabled() const;
+
   // Returns the owner window.
   v8::Local<v8::Value> GetOwnerBrowserWindow() const;
 
@@ -467,6 +469,9 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // Whether to enable devtools.
   bool enable_devtools_ = true;
 
+  // Whether to disable remote.
+  bool disable_remote_ = false;
+
   // Observers of this WebContents.
   base::ObserverList<ExtendedWebContentsObserver> observers_;
 

diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc
@@ -265,6 +265,10 @@ void WebContentsPreferences::AppendCommandLineSwitches(
     }
   }
 
+  // Disable the remote module
+  if (IsEnabled(options::kDisableRemote))
+    command_line->AppendSwitch(switches::kDisableRemote);
+
   // Run Electron APIs and preload script in isolated world
   if (IsEnabled(options::kContextIsolation))
     command_line->AppendSwitch(switches::kContextIsolation);

diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc
@@ -110,6 +110,9 @@ const char kPreloadURL[] = "preloadURL";
 // Enable the node integration.
 const char kNodeIntegration[] = "nodeIntegration";
 
+// Disable the remote module
+const char kDisableRemote[] = "disableRemote";
+
 // Enable context isolation of Electron APIs and preload script
 const char kContextIsolation[] = "contextIsolation";
 
@@ -193,6 +196,7 @@ const char kBackgroundColor[] = "background-color";
 const char kPreloadScript[] = "preload";
 const char kPreloadScripts[] = "preload-scripts";
 const char kNodeIntegration[] = "node-integration";
+const char kDisableRemote[] = "disable-remote";
 const char kContextIsolation[] = "context-isolation";
 const char kGuestInstanceID[] = "guest-instance-id";
 const char kOpenerID[] = "opener-id";

diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h
@@ -58,6 +58,7 @@ extern const char kZoomFactor[];
 extern const char kPreloadScript[];
 extern const char kPreloadURL[];
 extern const char kNodeIntegration[];
+extern const char kDisableRemote[];
 extern const char kContextIsolation[];
 extern const char kGuestInstanceID[];
 extern const char kExperimentalFeatures[];
@@ -97,6 +98,7 @@ extern const char kBackgroundColor[];
 extern const char kPreloadScript[];
 extern const char kPreloadScripts[];
 extern const char kNodeIntegration[];
+extern const char kDisableRemote[];
 extern const char kContextIsolation[];
 extern const char kGuestInstanceID[];
 extern const char kOpenerID[];

diff --git a/atom/renderer/renderer_client_base.cc b/atom/renderer/renderer_client_base.cc
@@ -78,6 +78,15 @@ std::vector<std::string> ParseSchemesCLISwitch(const char* switch_name) {
                            base::SPLIT_WANT_NONEMPTY);
 }
 
+void SetHiddenValue(v8::Handle<v8::Context> context,
+                    const base::StringPiece& key,
+                    v8::Local<v8::Value> value) {
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Local<v8::Private> privateKey =
+      v8::Private::ForApi(isolate, mate::StringToV8(isolate, key));
+  context->Global()->SetPrivate(context, privateKey, value);
+}
+
 }  // namespace
 
 RendererClientBase::RendererClientBase() {
@@ -99,10 +108,12 @@ void RendererClientBase::DidCreateScriptContext(
   std::string context_id = base::StringPrintf(
       "%" CrPRIdPid "-%d", base::GetCurrentProcId(), ++next_context_id_);
   v8::Isolate* isolate = context->GetIsolate();
-  v8::Local<v8::String> key = mate::StringToSymbol(isolate, "contextId");
-  v8::Local<v8::Private> private_key = v8::Private::ForApi(isolate, key);
-  v8::Local<v8::Value> value = mate::ConvertToV8(isolate, context_id);
-  context->Global()->SetPrivate(context, private_key, value);
+  SetHiddenValue(context, "contextId", mate::ConvertToV8(isolate, context_id));
+
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kDisableRemote)) {
+    SetHiddenValue(context, "disableRemote", mate::ConvertToV8(isolate, true));
+  }
 }
 
 void RendererClientBase::AddRenderBindings(

diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md
@@ -270,6 +270,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
       are more limited. Read more about the option [here](sandbox-option.md).
       **Note:** This option is currently experimental and may change or be
       removed in future Electron releases.
+    * `disableRemote` Boolean (optional) - If set, this will disable the [`remote`](remote.md) module.
     * `session` [Session](session.md#class-session) (optional) - Sets the session used by the
       page. Instead of passing the Session object directly, you can also choose to
       use the `partition` option instead, which accepts a partition string. When

diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js
@@ -1,8 +1,11 @@
 'use strict'
 
+const {spawn} = require('child_process')
 const electron = require('electron')
 const {EventEmitter} = require('events')
 const fs = require('fs')
+const os = require('os')
+const path = require('path')
 const v8Util = process.atomBinding('v8_util')
 
 const {ipcMain, isPromise} = electron
@@ -260,10 +263,14 @@ const callFunction = function (event, contextId, func, caller, args) {
 const handleRemoteCommand = function (channel, handler) {
   ipcMain.on(channel, (event, contextId, ...args) => {
     let returnValue
-    try {
-      returnValue = handler(event, contextId, ...args)
-    } catch (error) {
-      returnValue = exceptionToMeta(event.sender, contextId, error)
+    if (event.sender._isRemoteDisabled()) {
+      returnValue = exceptionToMeta(event.sender, contextId, new Error('remote is disabled'))
+    } else {
+      try {
+        returnValue = handler(event, contextId, ...args)
+      } catch (error) {
+        returnValue = exceptionToMeta(event.sender, contextId, error)
+      }
     }
     if (returnValue !== undefined) {
       event.returnValue = returnValue
@@ -396,8 +403,40 @@ ipcMain.on('ELECTRON_BROWSER_WINDOW_CLOSE', function (event) {
   event.returnValue = null
 })
 
+const getTempDirectory = function () {
+  try {
+    return electron.app.getPath('temp')
+  } catch (error) {
+    return os.tmpdir()
+  }
+}
+
+ipcMain.on('ELECTRON_CRASH_REPORTER_INIT', function (event, options) {
+  let crashesDirectory = path.join(getTempDirectory(), `${options.productName} Crashes`)
+
+  if (process.platform === 'win32') {
+    const env = {
+      ELECTRON_INTERNAL_CRASH_SERVICE: 1
+    }
+    const args = [
+      '--reporter-url=' + options.submitURL,
+      '--application-name=' + options.productName,
+      '--crashes-directory=' + crashesDirectory,
+      '--v=1'
+    ]
+
+    spawn(process.helperExecPath, args, {
+      env: env,
+      detached: true
+    })
+  }
+
+  event.returnValue = crashesDirectory
+})
+
 ipcMain.on('ELECTRON_BROWSER_SANDBOX_LOAD', function (event) {
   const preloadPath = event.sender._getPreloadPath()
+  const isRemoteDisabled = event.sender._isRemoteDisabled()
   let preloadSrc = null
   let preloadError = null
   if (preloadPath) {
@@ -408,8 +447,9 @@ ipcMain.on('ELECTRON_BROWSER_SANDBOX_LOAD', function (event) {
     }
   }
   event.returnValue = {
-    preloadSrc: preloadSrc,
-    preloadError: preloadError,
+    preloadSrc,
+    preloadError,
+    isRemoteDisabled,
     process: {
       arch: process.arch,
       platform: process.platform,

diff --git a/lib/common/api/clipboard.js b/lib/common/api/clipboard.js
@@ -1,14 +1,20 @@
+const {remote} = require('electron')
+
+if (!remote) {
+  throw new Error('clipboard requires remote, which is disabled')
+}
+
 if (process.platform === 'linux' && process.type === 'renderer') {
   // On Linux we could not access clipboard in renderer process.
-  module.exports = require('electron').remote.clipboard
+  module.exports = remote.clipboard
 } else {
   const clipboard = process.atomBinding('clipboard')
 
   // Read/write to find pasteboard over IPC since only main process is notified
   // of changes
   if (process.platform === 'darwin' && process.type === 'renderer') {
-    clipboard.readFindText = require('electron').remote.clipboard.readFindText
-    clipboard.writeFindText = require('electron').remote.clipboard.writeFindText
+    clipboard.readFindText = remote.clipboard.readFindText
+    clipboard.writeFindText = remote.clipboard.writeFindText
   }
 
   module.exports = clipboard

diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js
@@ -1,61 +1,56 @@
 'use strict'
 
-const {spawn} = require('child_process')
-const os = require('os')
-const path = require('path')
 const electron = require('electron')
-const {app} = process.type === 'browser' ? electron : electron.remote
+const {app} = process.type === 'browser' ? electron : electron.remote || {}
 const binding = process.atomBinding('crash_reporter')
 
+const getAppName = () => app && app.getName()
+const getAppVersion = () => app && app.getVersion()
+
 class CrashReporter {
   start (options) {
     if (options == null) options = {}
-    this.productName = options.productName != null ? options.productName : app.getName()
 
     let {
+      productName,
       companyName,
       extra,
       ignoreSystemCrashHandler,
       submitURL,
       uploadToServer
     } = options
 
+    if (productName == null) {
+      productName = getAppName()
+    }
+
     if (uploadToServer == null) {
       uploadToServer = true
     }
 
     if (ignoreSystemCrashHandler == null) ignoreSystemCrashHandler = false
     if (extra == null) extra = {}
 
-    if (extra._productName == null) extra._productName = this.getProductName()
+    if (extra._productName == null) extra._productName = productName
     if (extra._companyName == null) extra._companyName = companyName
-    if (extra._version == null) extra._version = app.getVersion()
+    if (extra._version == null) extra._version = getAppVersion()
 
+    if (productName == null) {
+      throw new Error('productName is a required option to crashReporter.start')
+    }
     if (companyName == null) {
       throw new Error('companyName is a required option to crashReporter.start')
     }
     if (submitURL == null) {
       throw new Error('submitURL is a required option to crashReporter.start')
     }
 
-    if (process.platform === 'win32') {
-      const env = {
-        ELECTRON_INTERNAL_CRASH_SERVICE: 1
-      }
-      const args = [
-        '--reporter-url=' + submitURL,
-        '--application-name=' + this.getProductName(),
-        '--crashes-directory=' + this.getCrashesDirectory(),
-        '--v=1'
-      ]
-
-      this._crashServiceProcess = spawn(process.execPath, args, {
-        env: env,
-        detached: true
-      })
-    }
+    this.crashesDirectory = electron.ipcRenderer.sendSync('ELECTRON_CRASH_REPORTER_INIT', {
+      submitURL,
+      productName
+    })
 
-    binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), uploadToServer, ignoreSystemCrashHandler, extra)
+    binding.start(productName, companyName, submitURL, this.crashesDirectory, uploadToServer, ignoreSystemCrashHandler, extra)
   }
 
   getLastCrashReport () {
@@ -70,30 +65,7 @@ class CrashReporter {
   }
 
   getUploadedReports () {
-    return binding.getUploadedReports(this.getCrashesDirectory())
-  }
-
-  getCrashesDirectory () {
-    const crashesDir = `${this.getProductName()} Crashes`
-    return path.join(this.getTempDirectory(), crashesDir)
-  }
-
-  getProductName () {
-    if (this.productName == null) {
-      this.productName = app.getName()
-    }
-    return this.productName
-  }
-
-  getTempDirectory () {
-    if (this.tempDirectory == null) {
-      try {
-        this.tempDirectory = app.getPath('temp')
-      } catch (error) {
-        this.tempDirectory = os.tmpdir()
-      }
-    }
-    return this.tempDirectory
+    return binding.getUploadedReports(this.crashesDirectory)
   }
 
   getUploadToServer () {

diff --git a/lib/renderer/api/module-list.js b/lib/renderer/api/module-list.js
@@ -1,4 +1,5 @@
 const features = process.atomBinding('features')
+const v8Util = process.atomBinding('v8_util')
 
 // Renderer side modules, please sort alphabetically.
 // A module is `enabled` if there is no explicit condition defined.
@@ -9,7 +10,11 @@ module.exports = [
     enabled: features.isDesktopCapturerEnabled()
   },
   {name: 'ipcRenderer', file: 'ipc-renderer', enabled: true},
-  {name: 'remote', file: 'remote', enabled: true},
+  {
+    name: 'remote',
+    file: 'remote',
+    enabled: v8Util.getHiddenValue(global, 'disableRemote') !== true
+  },
   {name: 'screen', file: 'screen', enabled: true},
   {name: 'webFrame', file: 'web-frame', enabled: true}
 ]
diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js
@@ -269,8 +269,6 @@ function metaToPlainObject (meta) {
 // Construct an exception error from the meta.
 function metaToException (meta) {
   const error = new Error(`${meta.message}\n${meta.stack}`)
-  const remoteProcess = exports.process
-  error.from = remoteProcess ? remoteProcess.type : null
   error.cause = metaToValue(meta.cause)
   return error
 }

diff --git a/lib/renderer/api/screen.js b/lib/renderer/api/screen.js
@@ -1 +1,7 @@
-module.exports = require('electron').remote.screen
+const {remote} = require('electron')
+
+if (!remote) {
+  throw new Error('screen requires remote, which is disabled')
+}
+
+module.exports = remote.screen
diff --git a/lib/sandboxed_renderer/api/exports/child_process.js b/lib/sandboxed_renderer/api/exports/child_process.js
@@ -1 +1 @@
-module.exports = require('electron').remote.require('child_process')
+module.exports = require('../remote-require')('child_process')