feat: sandbox renderer processes for cross-origin frames

shell/app/atom_main_delegate.cc

@@ -59,6 +59,11 @@ bool IsBrowserProcess(base::CommandLine* cmd) {
   return process_type.empty();
 }
 
+bool IsSandboxEnabled(base::CommandLine* command_line) {
+  return command_line->HasSwitch(switches::kEnableSandbox) ||
+         !command_line->HasSwitch(service_manager::switches::kNoSandbox);
+}
+
 // Returns true if this subprocess type needs the ResourceBundle initialized
 // and resources loaded.
 bool SubprocessNeedsResourceBundle(const std::string& process_type) {
@@ -281,10 +286,9 @@ content::ContentGpuClient* AtomMainDelegate::CreateContentGpuClient() {
 
 content::ContentRendererClient*
 AtomMainDelegate::CreateContentRendererClient() {
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableSandbox) ||
-      !base::CommandLine::ForCurrentProcess()->HasSwitch(
-          service_manager::switches::kNoSandbox)) {
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+
+  if (IsSandboxEnabled(command_line)) {
     renderer_client_.reset(new AtomSandboxedRendererClient);
   } else {
     renderer_client_.reset(new AtomRendererClient);

shell/browser/api/atom_api_web_contents.cc

@@ -1213,6 +1213,18 @@ base::ProcessId WebContents::GetOSProcessID() const {
   return base::GetProcId(process_handle);
 }
 
+base::ProcessId WebContents::GetOSProcessIdForFrame(
+    const std::string& name,
+    const std::string& document_url) const {
+  for (auto* frame : web_contents()->GetAllFrames()) {
+    if (frame->GetFrameName() == name &&
+        frame->GetLastCommittedURL().spec() == document_url) {
+      return base::GetProcId(frame->GetProcess()->GetProcess().Handle());
+    }
+  }
+  return base::kNullProcessId;
+}
+
 WebContents::Type WebContents::GetType() const {
   return type_;
 }
@@ -2194,6 +2206,8 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
                  &WebContents::SetBackgroundThrottling)
       .SetMethod("getProcessId", &WebContents::GetProcessID)
       .SetMethod("getOSProcessId", &WebContents::GetOSProcessID)
+      .SetMethod("_getOSProcessIdForFrame",
+                 &WebContents::GetOSProcessIdForFrame)
       .SetMethod("equal", &WebContents::Equal)
       .SetMethod("_loadURL", &WebContents::LoadURL)
       .SetMethod("downloadURL", &WebContents::DownloadURL)

shell/browser/api/atom_api_web_contents.h

@@ -138,6 +138,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
   void SetBackgroundThrottling(bool allowed);
   int GetProcessID() const;
   base::ProcessId GetOSProcessID() const;
+  base::ProcessId GetOSProcessIdForFrame(const std::string& name,
+                                         const std::string& document_url) const;
   Type GetType() const;
   bool Equal(const WebContents* web_contents) const;
   void LoadURL(const GURL& url, const mate::Dictionary& options);

shell/browser/atom_browser_client.cc

@@ -327,6 +327,10 @@ void AtomBrowserClient::ConsiderSiteInstanceForAffinity(
   }
 }
 
+bool AtomBrowserClient::IsRendererSubFrame(int process_id) const {
+  return base::ContainsKey(renderer_is_subframe_, process_id);
+}
+
 void AtomBrowserClient::RenderProcessWillLaunch(
     content::RenderProcessHost* host,
     service_manager::mojom::ServiceRequest* service_request) {
@@ -463,6 +467,11 @@ void AtomBrowserClient::RegisterPendingSiteInstance(
   auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
   auto* pending_process = pending_site_instance->GetProcess();
   pending_processes_[pending_process->GetID()] = web_contents;
+
+  if (rfh->GetParent())
+    renderer_is_subframe_.insert(pending_process->GetID());
+  else
+    renderer_is_subframe_.erase(pending_process->GetID());
 }
 
 void AtomBrowserClient::AppendExtraCommandLineSwitches(
@@ -513,7 +522,8 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
     }
     auto* web_preferences = WebContentsPreferences::From(web_contents);
     if (web_preferences)
-      web_preferences->AppendCommandLineSwitches(command_line);
+      web_preferences->AppendCommandLineSwitches(
+          command_line, IsRendererSubFrame(process_id));
     SessionPreferences::AppendExtraCommandLineSwitches(
         web_contents->GetBrowserContext(), command_line);
     if (CanUseCustomSiteInstance()) {
@@ -752,6 +762,7 @@ void AtomBrowserClient::RenderProcessHostDestroyed(
     content::RenderProcessHost* host) {
   int process_id = host->GetID();
   pending_processes_.erase(process_id);
+  renderer_is_subframe_.erase(process_id);
   RemoveProcessPreferences(process_id);
 }
 

shell/browser/atom_browser_client.h

@@ -235,11 +235,15 @@ class AtomBrowserClient : public content::ContentBrowserClient,
   void ConsiderSiteInstanceForAffinity(content::RenderFrameHost* rfh,
                                        content::SiteInstance* site_instance);
 
+  bool IsRendererSubFrame(int process_id) const;
+
   // pending_render_process => web contents.
   std::map<int, content::WebContents*> pending_processes_;
 
   std::map<int, base::ProcessId> render_process_host_pids_;
 
+  std::set<int> renderer_is_subframe_;
+
   // list of site per affinity. weak_ptr to prevent instance locking
   std::map<std::string, content::SiteInstance*> site_per_affinities_;
 

shell/browser/web_contents_preferences.cc

@@ -271,7 +271,8 @@ WebContentsPreferences* WebContentsPreferences::From(
 }
 
 void WebContentsPreferences::AppendCommandLineSwitches(
-    base::CommandLine* command_line) {
+    base::CommandLine* command_line,
+    bool is_subframe) {
   // Check if plugins are enabled.
   if (IsEnabled(options::kPlugins))
     command_line->AppendSwitch(switches::kEnablePlugins);
@@ -293,10 +294,16 @@ void WebContentsPreferences::AppendCommandLineSwitches(
   if (IsEnabled(options::kWebviewTag))
     command_line->AppendSwitch(switches::kWebviewTag);
 
+  // Sandbox can be enabled for renderer processes hosting cross-origin frames
+  // unless nodeIntegrationInSubFrames is enabled
+  bool can_sandbox_frame =
+      is_subframe && !IsEnabled(options::kNodeIntegrationInSubFrames);
+
   // 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 (IsEnabled(options::kSandbox)) {
+  // integration. Otherwise disable Chromium sandbox, unless app.enableSandbox()
+  // was called.
+  if (IsEnabled(options::kSandbox) || can_sandbox_frame) {
     command_line->AppendSwitch(switches::kEnableSandbox);
   } else if (!command_line->HasSwitch(switches::kEnableSandbox)) {
     command_line->AppendSwitch(service_manager::switches::kNoSandbox);

shell/browser/web_contents_preferences.h

@@ -47,7 +47,8 @@ class WebContentsPreferences
   void Merge(const base::DictionaryValue& new_web_preferences);
 
   // Append command paramters according to preferences.
-  void AppendCommandLineSwitches(base::CommandLine* command_line);
+  void AppendCommandLineSwitches(base::CommandLine* command_line,
+                                 bool is_subframe);
 
   // Modify the WebPreferences according to preferences.
   void OverrideWebkitPrefs(content::WebPreferences* prefs);

spec/api-subframe-spec.js

@@ -1,11 +1,12 @@
 const { expect } = require('chai')
 const { remote } = require('electron')
 const path = require('path')
+const http = require('http')
 
 const { emittedNTimes, emittedOnce } = require('./events-helpers')
 const { closeWindow } = require('./window-helpers')
 
-const { BrowserWindow } = remote
+const { app, BrowserWindow, ipcMain } = remote
 
 describe('renderer nodeIntegrationInSubFrames', () => {
   const generateTests = (description, webPreferences) => {
@@ -149,3 +150,88 @@ describe('renderer nodeIntegrationInSubFrames', () => {
     generateTests(config.title, config.webPreferences)
   })
 })
+
+describe('cross-site frame sandboxing', () => {
+  let server = null
+
+  beforeEach(function () {
+    if (process.platform === 'linux') {
+      this.skip()
+    }
+  })
+
+  before(function (done) {
+    server = http.createServer((req, res) => {
+      res.end(`<iframe name="frame" src="${server.cross_site_url}" />`)
+    })
+    server.listen(0, '127.0.0.1', () => {
+      server.url = `http://127.0.0.1:${server.address().port}/`
+      server.cross_site_url = `http://localhost:${server.address().port}/`
+      done()
+    })
+  })
+
+  after(() => {
+    server.close()
+    server = null
+  })
+
+  const fixtures = path.resolve(__dirname, 'fixtures')
+  const preload = path.join(fixtures, 'module', 'preload-pid.js')
+
+  let w
+
+  afterEach(() => {
+    return closeWindow(w).then(() => {
+      w = null
+    })
+  })
+
+  const generateSpecs = (description, webPreferences) => {
+    describe(description, () => {
+      it('iframe process is sandboxed if possible', async () => {
+        w = new BrowserWindow({
+          show: false,
+          webPreferences
+        })
+
+        await w.loadURL(server.url)
+
+        const pidMain = w.webContents.getOSProcessId()
+        const pidFrame = w.webContents._getOSProcessIdForFrame('frame', server.cross_site_url)
+
+        const metrics = app.getAppMetrics()
+        const isProcessSandboxed = function (pid) {
+          const entry = metrics.filter(metric => metric.pid === pid)[0]
+          return entry && entry.sandboxed
+        }
+
+        const sandboxMain = !!(webPreferences.sandbox || process.mas)
+        const sandboxFrame = sandboxMain || !webPreferences.nodeIntegrationInSubFrames
+
+        expect(isProcessSandboxed(pidMain)).to.equal(sandboxMain)
+        expect(isProcessSandboxed(pidFrame)).to.equal(sandboxFrame)
+      })
+    })
+  }
+
+  generateSpecs('nodeIntegrationInSubFrames = false, sandbox = false', {
+    nodeIntegrationInSubFrames: false,
+    sandbox: false
+  })
+
+  generateSpecs('nodeIntegrationInSubFrames = false, sandbox = true', {
+    nodeIntegrationInSubFrames: false,
+    sandbox: true
+  })
+
+  generateSpecs('nodeIntegrationInSubFrames = true, sandbox = false', {
+    nodeIntegrationInSubFrames: true,
+    sandbox: false
+  })
+
+  generateSpecs('nodeIntegrationInSubFrames = true, sandbox = true', {
+    nodeIntegrationInSubFrames: true,
+    sandbox: true
+  })
+})