From 3dcd059b7c6e2edcb152a04bc53a247961d529a9 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 21 May 2018 23:07:00 +0200 Subject: [PATCH] Add webPreferences.disableRemote option to disable the remote module (sandbox security) --- atom/browser/api/atom_api_web_contents.cc | 14 ++++++-- atom/browser/api/atom_api_web_contents.h | 5 +++ docs/api/browser-window.md | 1 + lib/browser/rpc-server.js | 39 +++++++++++++++++++++++ lib/renderer/api/remote.js | 2 -- 5 files changed, 57 insertions(+), 4 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index c79f18d7f1c11..59b4d0dcb2670 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -342,7 +342,8 @@ WebContents::WebContents(v8::Isolate* isolate, type_(type), request_id_(0), background_throttling_(true), - enable_devtools_(true) { + enable_devtools_(true), + disable_remote_(false) { const mate::Dictionary options = mate::Dictionary::CreateEmpty(isolate); if (type == REMOTE) { web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent()); @@ -362,7 +363,8 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options) type_(BROWSER_WINDOW), request_id_(0), background_throttling_(true), - enable_devtools_(true) { + enable_devtools_(true), + disable_remote_(false) { // Read options. options.Get("backgroundThrottling", &background_throttling_); @@ -389,6 +391,9 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options) // 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 session; @@ -1922,6 +1927,10 @@ v8::Local WebContents::GetLastWebPreferences(v8::Isolate* isolate) { return mate::ConvertToV8(isolate, *web_preferences->last_dict()); } +bool WebContents::IsRemoteDisabled() const { + return disable_remote_; +} + v8::Local WebContents::GetOwnerBrowserWindow() { if (owner_window()) return BrowserWindow::From(isolate(), owner_window()); @@ -2070,6 +2079,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate, .SetMethod("getType", &WebContents::GetType) .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 index 84c4e3eccba55..aa7a063b44c07 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -234,6 +234,8 @@ class WebContents : public mate::TrackableObject, v8::Local GetWebPreferences(v8::Isolate* isolate); v8::Local GetLastWebPreferences(v8::Isolate* isolate); + bool IsRemoteDisabled() const; + // Returns the owner window. v8::Local GetOwnerBrowserWindow(); @@ -454,6 +456,9 @@ class WebContents : public mate::TrackableObject, // Whether to enable devtools. bool enable_devtools_; + // Whether to disable remote. + bool disable_remote_; + // Observers of this WebContents. base::ObserverList observers_; diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index a6dfe1d658762..d87e744e22a13 100644 --- 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 index 5784d2fd31e9e..e0197c6f3c071 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -261,7 +261,18 @@ const callFunction = function (event, func, caller, args) { } } +const isRemoteDisabled = (event) => { + if (event.sender._isRemoteDisabled()) { + event.returnValue = exceptionToMeta(event.sender, new Error('remote is disabled')) + return true + } + + return false +} + ipcMain.on('ELECTRON_BROWSER_REQUIRE', function (event, module) { + if (isRemoteDisabled(event)) return + try { event.returnValue = valueToMeta(event.sender, process.mainModule.require(module)) } catch (error) { @@ -270,6 +281,8 @@ ipcMain.on('ELECTRON_BROWSER_REQUIRE', function (event, module) { }) ipcMain.on('ELECTRON_BROWSER_GET_BUILTIN', function (event, module) { + if (isRemoteDisabled(event)) return + try { event.returnValue = valueToMeta(event.sender, electron[module]) } catch (error) { @@ -278,6 +291,8 @@ ipcMain.on('ELECTRON_BROWSER_GET_BUILTIN', function (event, module) { }) ipcMain.on('ELECTRON_BROWSER_GLOBAL', function (event, name) { + if (isRemoteDisabled(event)) return + try { event.returnValue = valueToMeta(event.sender, global[name]) } catch (error) { @@ -286,6 +301,8 @@ ipcMain.on('ELECTRON_BROWSER_GLOBAL', function (event, name) { }) ipcMain.on('ELECTRON_BROWSER_CURRENT_WINDOW', function (event) { + if (isRemoteDisabled(event)) return + try { event.returnValue = valueToMeta(event.sender, event.sender.getOwnerBrowserWindow()) } catch (error) { @@ -294,10 +311,14 @@ ipcMain.on('ELECTRON_BROWSER_CURRENT_WINDOW', function (event) { }) ipcMain.on('ELECTRON_BROWSER_CURRENT_WEB_CONTENTS', function (event) { + if (isRemoteDisabled(event)) return + event.returnValue = valueToMeta(event.sender, event.sender) }) ipcMain.on('ELECTRON_BROWSER_CONSTRUCTOR', function (event, id, args) { + if (isRemoteDisabled(event)) return + try { args = unwrapArgs(event.sender, args) let constructor = objectsRegistry.get(id) @@ -316,6 +337,8 @@ ipcMain.on('ELECTRON_BROWSER_CONSTRUCTOR', function (event, id, args) { }) ipcMain.on('ELECTRON_BROWSER_FUNCTION_CALL', function (event, id, args) { + if (isRemoteDisabled(event)) return + try { args = unwrapArgs(event.sender, args) let func = objectsRegistry.get(id) @@ -331,6 +354,8 @@ ipcMain.on('ELECTRON_BROWSER_FUNCTION_CALL', function (event, id, args) { }) ipcMain.on('ELECTRON_BROWSER_MEMBER_CONSTRUCTOR', function (event, id, method, args) { + if (isRemoteDisabled(event)) return + try { args = unwrapArgs(event.sender, args) let object = objectsRegistry.get(id) @@ -349,6 +374,8 @@ ipcMain.on('ELECTRON_BROWSER_MEMBER_CONSTRUCTOR', function (event, id, method, a }) ipcMain.on('ELECTRON_BROWSER_MEMBER_CALL', function (event, id, method, args) { + if (isRemoteDisabled(event)) return + try { args = unwrapArgs(event.sender, args) let obj = objectsRegistry.get(id) @@ -364,6 +391,8 @@ ipcMain.on('ELECTRON_BROWSER_MEMBER_CALL', function (event, id, method, args) { }) ipcMain.on('ELECTRON_BROWSER_MEMBER_SET', function (event, id, name, args) { + if (isRemoteDisabled(event)) return + try { args = unwrapArgs(event.sender, args) let obj = objectsRegistry.get(id) @@ -380,6 +409,8 @@ ipcMain.on('ELECTRON_BROWSER_MEMBER_SET', function (event, id, name, args) { }) ipcMain.on('ELECTRON_BROWSER_MEMBER_GET', function (event, id, name) { + if (isRemoteDisabled(event)) return + try { let obj = objectsRegistry.get(id) @@ -394,15 +425,21 @@ ipcMain.on('ELECTRON_BROWSER_MEMBER_GET', function (event, id, name) { }) ipcMain.on('ELECTRON_BROWSER_DEREFERENCE', function (event, id) { + if (isRemoteDisabled(event)) return + objectsRegistry.remove(event.sender.getId(), id) }) ipcMain.on('ELECTRON_BROWSER_CONTEXT_RELEASE', (e, contextId) => { + if (isRemoteDisabled(event)) return + objectsRegistry.clear(contextId) e.returnValue = null }) ipcMain.on('ELECTRON_BROWSER_GUEST_WEB_CONTENTS', function (event, guestInstanceId) { + if (isRemoteDisabled(event)) return + try { let guestViewManager = require('./guest-view-manager') event.returnValue = valueToMeta(event.sender, guestViewManager.getGuest(guestInstanceId)) @@ -412,6 +449,8 @@ ipcMain.on('ELECTRON_BROWSER_GUEST_WEB_CONTENTS', function (event, guestInstance }) ipcMain.on('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', function (event, requestId, guestInstanceId, method, ...args) { + if (isRemoteDisabled(event)) return + try { let guestViewManager = require('./guest-view-manager') let guest = guestViewManager.getGuest(guestInstanceId) diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index e1f7156f1ce01..cfd097d6db59d 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -259,8 +259,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 }