From cd5e900374e9f2810cf2b550f6610a4356115cbb Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Wed, 5 Jun 2019 13:40:47 +0200 Subject: [PATCH] feat: sandbox renderer processes for cross-origin frames --- atom/app/atom_main_delegate.cc | 12 ++-- atom/browser/api/atom_api_web_contents.cc | 40 ++++++++--- atom/browser/api/atom_api_web_contents.h | 4 ++ atom/browser/atom_browser_client.cc | 13 +++- atom/browser/atom_browser_client.h | 4 ++ atom/browser/web_contents_preferences.cc | 13 +++- atom/browser/web_contents_preferences.h | 3 +- spec/api-subframe-spec.js | 88 ++++++++++++++++++++++- 8 files changed, 159 insertions(+), 18 deletions(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 95c655999add5..28fff3296fdcf 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/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); diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index f48210a8b9da3..d1abf4d27e33b 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1213,6 +1213,14 @@ base::ProcessId WebContents::GetOSProcessID() const { return base::GetProcId(process_handle); } +base::ProcessId WebContents::GetOSProcessIdForFrame( + const std::string& name) const { + if (auto* frame = GetFrameByName(name)) { + return base::GetProcId(frame->GetProcess()->GetProcess().Handle()); + } + return base::kNullProcessId; +} + WebContents::Type WebContents::GetType() const { return type_; } @@ -1735,17 +1743,12 @@ bool WebContents::SendIPCMessageToFrame(bool internal, int32_t frame_id, const std::string& channel, const base::ListValue& args) { - auto frames = web_contents()->GetAllFrames(); - auto iter = std::find_if(frames.begin(), frames.end(), [frame_id](auto* f) { - return f->GetRoutingID() == frame_id; - }); - if (iter == frames.end()) - return false; - if (!(*iter)->IsRenderFrameLive()) + auto* frame = GetFrameByRoutingID(frame_id); + if (!frame || !frame->IsRenderFrameLive()) return false; mojom::ElectronRendererAssociatedPtr electron_ptr; - (*iter)->GetRemoteAssociatedInterfaces()->GetInterface( + frame->GetRemoteAssociatedInterfaces()->GetInterface( mojo::MakeRequest(&electron_ptr)); electron_ptr->Message(internal, send_to_all, channel, args.Clone(), 0 /* sender_id */); @@ -2194,6 +2197,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) @@ -2298,6 +2303,25 @@ AtomBrowserContext* WebContents::GetBrowserContext() const { return static_cast(web_contents()->GetBrowserContext()); } +content::RenderFrameHost* WebContents::GetFrameByRoutingID(int frame_id) const { + auto frames = web_contents()->GetAllFrames(); + auto it = std::find_if(frames.begin(), frames.end(), [frame_id](auto* f) { + return f->GetRoutingID() == frame_id; + }); + + return (it != frames.end()) ? *it : nullptr; +} + +content::RenderFrameHost* WebContents::GetFrameByName( + const std::string& name) const { + auto frames = web_contents()->GetAllFrames(); + auto it = std::find_if(frames.begin(), frames.end(), [name](auto* f) { + return !f->GetFrameName().empty() && f->GetFrameName() == name; + }); + + return (it != frames.end()) ? *it : nullptr; +} + // static mate::Handle WebContents::Create(v8::Isolate* isolate, const mate::Dictionary& options) { diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 39a7fb9c73d86..fef2c7e016adf 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -138,6 +138,7 @@ class WebContents : public mate::TrackableObject, void SetBackgroundThrottling(bool allowed); int GetProcessID() const; base::ProcessId GetOSProcessID() const; + base::ProcessId GetOSProcessIdForFrame(const std::string& name) const; Type GetType() const; bool Equal(const WebContents* web_contents) const; void LoadURL(const GURL& url, const mate::Dictionary& options); @@ -477,6 +478,9 @@ class WebContents : public mate::TrackableObject, private: AtomBrowserContext* GetBrowserContext() const; + content::RenderFrameHost* GetFrameByRoutingID(int frame_id) const; + content::RenderFrameHost* GetFrameByName(const std::string& name) const; + // Binds the given request for the ElectronBrowser API. When the // RenderFrameHost is destroyed, all related bindings will be removed. void BindElectronBrowser(mojom::ElectronBrowserRequest request, diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 736b0d6d69d37..7abcf80ef2647 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/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); } diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index 4cc72e139b15e..ef3bc34fde51e 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/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 pending_processes_; std::map render_process_host_pids_; + std::set renderer_is_subframe_; + // list of site per affinity. weak_ptr to prevent instance locking std::map site_per_affinities_; diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index b7032f72b3694..215aba3ae08ef 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/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); diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index 465acf4a1569a..57c24a2827d35 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/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); diff --git a/spec/api-subframe-spec.js b/spec/api-subframe-spec.js index 3b9841fb23013..094ff6b53b6e8 100644 --- a/spec/api-subframe-spec.js +++ b/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(`