Merge pull request #9644 from electron/webview-sandbox

Enable sandbox on webview

atom/app/atom_main_delegate.cc

@@ -126,14 +126,16 @@ void AtomMainDelegate::PreSandboxStartup() {
   if (!IsBrowserProcess(command_line))
     return;
 
+  if (!command_line->HasSwitch(switches::kEnableMixedSandbox)) {
     if (command_line->HasSwitch(switches::kEnableSandbox)) {
-    // Disable setuid sandbox since it is not longer required on linux(namespace
+      // Disable setuid sandbox since it is not longer required on
-    // sandbox is available on most distros).
+      // linux(namespace sandbox is available on most distros).
       command_line->AppendSwitch(::switches::kDisableSetuidSandbox);
     } else {
       // Disable renderer sandbox for most of node's functions.
       command_line->AppendSwitch(::switches::kNoSandbox);
     }
+  }
 
   // Allow file:// URIs to read other file:// URIs by default.
   command_line->AppendSwitch(::switches::kAllowFileAccessFromFiles);

atom/browser/api/atom_api_app.cc

@@ -1033,6 +1033,33 @@ v8::Local<v8::Value> App::GetGPUFeatureStatus(v8::Isolate* isolate) {
                            status ? *status : base::DictionaryValue());
 }
 
+void App::EnableMixedSandbox(mate::Arguments* args) {
+  if (Browser::Get()->is_ready()) {
+    args->ThrowError("app.enableMixedSandbox() can only be called "
+                     "before app is ready");
+    return;
+  }
+
+  auto command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(::switches::kNoSandbox)) {
+#if defined(OS_WIN)
+    const base::CommandLine::CharType* noSandboxArg = L"--no-sandbox";
+#else
+    const base::CommandLine::CharType* noSandboxArg = "--no-sandbox";
+#endif
+
+    // Remove the --no-sandbox switch
+    base::CommandLine::StringVector modified_command_line;
+    for (auto& arg : command_line->argv()) {
+      if (arg.compare(noSandboxArg) != 0) {
+        modified_command_line.push_back(arg);
+      }
+    }
+    command_line->InitFromArgv(modified_command_line);
+  }
+  command_line->AppendSwitch(switches::kEnableMixedSandbox);
+}
+
 // static
 mate::Handle<App> App::Create(v8::Isolate* isolate) {
   return mate::CreateHandle(isolate, new App(isolate));
@@ -1107,6 +1134,7 @@ void App::BuildPrototype(
       .SetMethod("getFileIcon", &App::GetFileIcon)
       .SetMethod("getAppMetrics", &App::GetAppMetrics)
       .SetMethod("getGPUFeatureStatus", &App::GetGPUFeatureStatus)
+      .SetMethod("enableMixedSandbox", &App::EnableMixedSandbox)
       // TODO(juturu): Remove in 2.0, deprecate before then with warnings
       .SetMethod("getAppMemoryInfo", &App::GetAppMetrics);
 }

atom/browser/api/atom_api_app.h

@@ -180,6 +180,7 @@ class App : public AtomBrowserClient::Delegate,
 
   std::vector<mate::Dictionary> GetAppMetrics(v8::Isolate* isolate);
   v8::Local<v8::Value> GetGPUFeatureStatus(v8::Isolate* isolate);
+  void EnableMixedSandbox(mate::Arguments* args);
 
 #if defined(OS_WIN)
   // Get the current Jump List settings.

atom/browser/web_contents_preferences.cc

@@ -111,8 +111,11 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches(
   // If the `sandbox` option was passed to the BrowserWindow's webPreferences,
   // pass `--enable-sandbox` to the renderer so it won't have any node.js
   // integration.
-  if (IsSandboxed(web_contents))
+  if (IsSandboxed(web_contents)) {
     command_line->AppendSwitch(switches::kEnableSandbox);
+  } else if (!command_line->HasSwitch(switches::kEnableSandbox)) {
+    command_line->AppendSwitch(::switches::kNoSandbox);
+  }
   if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b)
     command_line->AppendSwitch(switches::kNativeWindowOpen);
 

atom/common/options_switches.cc

@@ -138,6 +138,9 @@ namespace switches {
 // Enable chromium sandbox.
 const char kEnableSandbox[] = "enable-sandbox";
 
+// Enable sandbox in only remote content windows.
+const char kEnableMixedSandbox[] = "enable-mixed-sandbox";
+
 // Enable plugins.
 const char kEnablePlugins[] = "enable-plugins";
 

atom/common/options_switches.h

@@ -74,6 +74,7 @@ extern const char kWebviewTag[];
 namespace switches {
 
 extern const char kEnableSandbox[];
+extern const char kEnableMixedSandbox[];
 extern const char kEnablePlugins[];
 extern const char kPpapiFlashPath[];
 extern const char kPpapiFlashVersion[];

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -80,6 +80,10 @@ v8::Local<v8::Value> GetBinding(v8::Isolate* isolate, v8::Local<v8::String> key,
   return exports;
 }
 
+base::CommandLine::StringVector GetArgv() {
+  return base::CommandLine::ForCurrentProcess()->argv();
+}
+
 void InitializeBindings(v8::Local<v8::Object> binding,
                         v8::Local<v8::Context> context) {
   auto isolate = context->GetIsolate();
@@ -87,6 +91,7 @@ void InitializeBindings(v8::Local<v8::Object> binding,
   b.SetMethod("get", GetBinding);
   b.SetMethod("crash", AtomBindings::Crash);
   b.SetMethod("hang", AtomBindings::Hang);
+  b.SetMethod("getArgv", GetArgv);
   b.SetMethod("getProcessMemoryInfo", &AtomBindings::GetProcessMemoryInfo);
   b.SetMethod("getSystemMemoryInfo", &AtomBindings::GetSystemMemoryInfo);
 }

docs/api/app.md

@@ -913,6 +913,12 @@ correctly.
 
 **Note:** This will not affect `process.argv`.
 
+### `app.enableMixedSandbox()` _Experimental_
+
+Enables mixed sandbox mode on the app.
+
+This method can only be called before app is ready.
+
 ### `app.dock.bounce([type])` _macOS_
 
 * `type` String (optional) - Can be `critical` or `informational`. The default is

docs/api/sandbox-option.md

@@ -60,6 +60,8 @@ It is important to note that this option alone won't enable the OS-enforced sand
 `--enable-sandbox` command-line argument must be passed to electron, which will
 force `sandbox: true` for all `BrowserWindow` instances.
 
+To enable OS-enforced sandbox on `BrowserWindow` or `webview` process with `sandbox:true` without causing
+entire app to be in sandbox, `--enable-mixed-sandbox` command-line argument must be passed to electron.
 
 ```js
 let win

lib/sandboxed_renderer/init.js

@@ -41,6 +41,7 @@ preloadProcess.crash = () => binding.crash()
 preloadProcess.hang = () => binding.hang()
 preloadProcess.getProcessMemoryInfo = () => binding.getProcessMemoryInfo()
 preloadProcess.getSystemMemoryInfo = () => binding.getSystemMemoryInfo()
+preloadProcess.argv = binding.getArgv()
 process.platform = preloadProcess.platform = electron.remote.process.platform
 process.execPath = preloadProcess.execPath = electron.remote.process.execPath
 process.on('exit', () => preloadProcess.emit('exit'))

spec/api-app-spec.js

@@ -566,4 +566,82 @@ describe('app module', function () {
       assert.equal(typeof features.gpu_compositing, 'string')
     })
   })
+
+  describe('mixed sandbox option', function () {
+    let appProcess = null
+    let server = null
+    const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox'
+
+    beforeEach(function (done) {
+      fs.unlink(socketPath, () => {
+        server = net.createServer()
+        server.listen(socketPath)
+        done()
+      })
+    })
+
+    afterEach(function (done) {
+      if (appProcess != null) {
+        appProcess.kill()
+      }
+
+      server.close(() => {
+        if (process.platform === 'win32') {
+          done()
+        } else {
+          fs.unlink(socketPath, () => {
+            done()
+          })
+        }
+      })
+    })
+
+    describe('when app.enableMixedSandbox() is called', () => {
+      it('adds --enable-sandbox to render processes created with sandbox: true', (done) => {
+        const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
+        appProcess = ChildProcess.spawn(remote.process.execPath, [appPath])
+
+        server.once('error', (error) => {
+          done(error)
+        })
+
+        server.on('connection', (client) => {
+          client.once('data', function (data) {
+            const argv = JSON.parse(data)
+            assert.equal(argv.sandbox.includes('--enable-sandbox'), true)
+            assert.equal(argv.sandbox.includes('--no-sandbox'), false)
+
+            assert.equal(argv.noSandbox.includes('--enable-sandbox'), false)
+            assert.equal(argv.noSandbox.includes('--no-sandbox'), true)
+
+            done()
+          })
+        })
+      })
+    })
+
+    describe('when the app is launched with --enable-mixed-sandbox', () => {
+      it('adds --enable-sandbox to render processes created with sandbox: true', (done) => {
+        const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
+        appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'])
+
+        server.once('error', (error) => {
+          done(error)
+        })
+
+        server.on('connection', (client) => {
+          client.once('data', function (data) {
+            const argv = JSON.parse(data)
+            assert.equal(argv.sandbox.includes('--enable-sandbox'), true)
+            assert.equal(argv.sandbox.includes('--no-sandbox'), false)
+
+            assert.equal(argv.noSandbox.includes('--enable-sandbox'), false)
+            assert.equal(argv.noSandbox.includes('--no-sandbox'), true)
+
+            done()
+          })
+        })
+      })
+    })
+  })
 })

spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js

@@ -0,0 +1 @@
+require('electron').ipcRenderer.send('argv', process.argv)

spec/fixtures/api/mixed-sandbox-app/main.js

@@ -0,0 +1,65 @@
+const {app, BrowserWindow, ipcMain} = require('electron')
+const net = require('net')
+const path = require('path')
+
+process.on('uncaughtException', () => {
+  app.exit(1)
+})
+
+if (!process.argv.includes('--enable-mixed-sandbox')) {
+  app.enableMixedSandbox()
+}
+
+let sandboxWindow
+let noSandboxWindow
+
+app.once('ready', () => {
+  sandboxWindow = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      preload: path.join(__dirname, 'electron-app-mixed-sandbox-preload.js'),
+      sandbox: true
+    }
+  })
+  sandboxWindow.loadURL('about:blank')
+
+  noSandboxWindow = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      preload: path.join(__dirname, 'electron-app-mixed-sandbox-preload.js'),
+      sandbox: false
+    }
+  })
+  noSandboxWindow.loadURL('about:blank')
+
+  const argv = {
+    sandbox: null,
+    noSandbox: null
+  }
+
+  let connected = false
+
+  function finish () {
+    if (connected && argv.sandbox != null && argv.noSandbox != null) {
+      client.once('end', () => {
+        app.exit(0)
+      })
+      client.end(JSON.stringify(argv))
+    }
+  }
+
+  const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox'
+  const client = net.connect(socketPath, () => {
+    connected = true
+    finish()
+  })
+
+  ipcMain.on('argv', (event, value) => {
+    if (event.sender === sandboxWindow.webContents) {
+      argv.sandbox = value
+    } else if (event.sender === noSandboxWindow.webContents) {
+      argv.noSandbox = value
+    }
+    finish()
+  })
+})

spec/fixtures/api/mixed-sandbox-app/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-app-mixed-sandbox",
+  "main": "main.js"
+}
+