diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index fca40a30cc04..61dbcf7b536b 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -1286,9 +1286,10 @@ void ElectronBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories( context, ukm_source_id, false /* we don't support extensions::WebViewGuest */)); #endif + // Always allow navigating to file:// URLs. auto* protocol_registry = ProtocolRegistry::FromBrowserContext(context); - protocol_registry->RegisterURLLoaderFactories( - URLLoaderFactoryType::kNavigation, factories); + protocol_registry->RegisterURLLoaderFactories(factories, + true /* allow_file_access */); } void ElectronBrowserClient:: @@ -1297,8 +1298,10 @@ void ElectronBrowserClient:: NonNetworkURLLoaderFactoryMap* factories) { auto* protocol_registry = ProtocolRegistry::FromBrowserContext(browser_context); - protocol_registry->RegisterURLLoaderFactories( - URLLoaderFactoryType::kWorkerMainResource, factories); + // Workers are not allowed to request file:// URLs, there is no particular + // reason for it, and we could consider supporting it in future. + protocol_registry->RegisterURLLoaderFactories(factories, + false /* allow_file_access */); } #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) @@ -1369,9 +1372,22 @@ void ElectronBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories( if (!render_process_host || !render_process_host->GetBrowserContext()) return; + content::RenderFrameHost* frame_host = + content::RenderFrameHost::FromID(render_process_id, render_frame_id); + content::WebContents* web_contents = + content::WebContents::FromRenderFrameHost(frame_host); + + // Allow accessing file:// subresources from non-file protocols if web + // security is disabled. + bool allow_file_access = false; + if (web_contents) { + const auto& web_preferences = web_contents->GetOrCreateWebPreferences(); + if (!web_preferences.web_security_enabled) + allow_file_access = true; + } + ProtocolRegistry::FromBrowserContext(render_process_host->GetBrowserContext()) - ->RegisterURLLoaderFactories(URLLoaderFactoryType::kDocumentSubResource, - factories); + ->RegisterURLLoaderFactories(factories, allow_file_access); #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) auto factory = extensions::CreateExtensionURLLoaderFactory(render_process_id, @@ -1379,10 +1395,6 @@ void ElectronBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories( if (factory) factories->emplace(extensions::kExtensionScheme, std::move(factory)); - content::RenderFrameHost* frame_host = - content::RenderFrameHost::FromID(render_process_id, render_frame_id); - content::WebContents* web_contents = - content::WebContents::FromRenderFrameHost(frame_host); if (!web_contents) return; diff --git a/shell/browser/protocol_registry.cc b/shell/browser/protocol_registry.cc index 60137d9db9b2..b703886de372 100644 --- a/shell/browser/protocol_registry.cc +++ b/shell/browser/protocol_registry.cc @@ -5,6 +5,7 @@ #include #include +#include "content/public/browser/web_contents.h" #include "services/network/public/cpp/self_deleting_url_loader_factory.h" #include "shell/browser/electron_browser_context.h" #include "shell/browser/net/asar/asar_url_loader.h" @@ -61,22 +62,20 @@ ProtocolRegistry::ProtocolRegistry() {} ProtocolRegistry::~ProtocolRegistry() = default; void ProtocolRegistry::RegisterURLLoaderFactories( - URLLoaderFactoryType type, - content::ContentBrowserClient::NonNetworkURLLoaderFactoryMap* factories) { - // Override the default FileURLLoaderFactory to support asar archives. - if (type == URLLoaderFactoryType::kNavigation) { - // Always allow navigating to file:// URLs. + content::ContentBrowserClient::NonNetworkURLLoaderFactoryMap* factories, + bool allow_file_access) { + auto file_factory = factories->find(url::kFileScheme); + if (file_factory != factories->end()) { + // If Chromium already allows file access then replace the url factory to + // also loading asar files. + file_factory->second = AsarURLLoaderFactory::Create(); + } else if (allow_file_access) { + // Otherwise only allow file access when it is explicitly allowed. // - // Note that Chromium calls |emplace| to create the default file factory - // after this call, so it won't override our asar factory. - DCHECK(!base::Contains(*factories, url::kFileScheme)); + // Note that Chromium may call |emplace| to create the default file factory + // after this call, it won't override our asar factory, but if asar support + // breaks in future, please check if Chromium has changed the call. factories->emplace(url::kFileScheme, AsarURLLoaderFactory::Create()); - } else if (type == URLLoaderFactoryType::kDocumentSubResource) { - // Only support requesting file:// subresource URLs when Chromium does so, - // it is usually supported under file:// or about:blank documents. - auto file_factory = factories->find(url::kFileScheme); - if (file_factory != factories->end()) - file_factory->second = AsarURLLoaderFactory::Create(); } for (const auto& it : handlers_) { diff --git a/shell/browser/protocol_registry.h b/shell/browser/protocol_registry.h index 7a58513061e4..ea988dc7f7a4 100644 --- a/shell/browser/protocol_registry.h +++ b/shell/browser/protocol_registry.h @@ -26,8 +26,8 @@ class ProtocolRegistry { content::ContentBrowserClient::URLLoaderFactoryType; void RegisterURLLoaderFactories( - URLLoaderFactoryType type, - content::ContentBrowserClient::NonNetworkURLLoaderFactoryMap* factories); + content::ContentBrowserClient::NonNetworkURLLoaderFactoryMap* factories, + bool allow_file_access); const HandlersMap& intercept_handlers() const { return intercept_handlers_; } diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index c019114785a4..bd0762b85652 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -287,6 +287,39 @@ describe('web security', () => { expect(response).to.equal('passed'); }); + describe('accessing file://', () => { + async function loadFile (w: BrowserWindow) { + const thisFile = url.format({ + pathname: __filename.replace(/\\/g, '/'), + protocol: 'file', + slashes: true + }); + await w.loadURL(`data:text/html,`); + return await w.webContents.executeJavaScript('loadFile()'); + } + + it('is forbidden when web security is enabled', async () => { + const w = new BrowserWindow({ show: false, webPreferences: { webSecurity: true } }); + const result = await loadFile(w); + expect(result).to.equal('failed'); + }); + + it('is allowed when web security is disabled', async () => { + const w = new BrowserWindow({ show: false, webPreferences: { webSecurity: false } }); + const result = await loadFile(w); + expect(result).to.equal('loaded'); + }); + }); + it('does not crash when multiple WebContent are created with web security disabled', () => { const options = { show: false, webPreferences: { webSecurity: false } }; const w1 = new BrowserWindow(options); diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 8fb6a1722ba9..668475bc73fe 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -33,6 +33,28 @@ describe(' tag', function () { return event.message; }; + async function loadFileInWebView (webview, attributes = {}) { + const thisFile = url.format({ + pathname: __filename.replace(/\\/g, '/'), + protocol: 'file', + slashes: true + }); + const src = ``; + attributes.src = `data:text/html;base64,${btoa(unescape(encodeURIComponent(src)))}`; + await startLoadingWebViewAndWaitForMessage(webview, attributes); + return await webview.executeJavaScript('loadFile()'); + } + beforeEach(() => { webview = new WebView(); }); @@ -321,27 +343,13 @@ describe(' tag', function () { describe('disablewebsecurity attribute', () => { it('does not disable web security when not set', async () => { - const jqueryPath = path.join(__dirname, '/static/jquery-2.0.3.min.js'); - const src = ` `; - const encoded = btoa(unescape(encodeURIComponent(src))); - - const message = await startLoadingWebViewAndWaitForMessage(webview, { - src: `data:text/html;base64,${encoded}` - }); - expect(message).to.be.a('string'); - expect(message).to.contain('Not allowed to load local resource'); + const result = await loadFileInWebView(webview); + expect(result).to.equal('failed'); }); it('disables web security when set', async () => { - const jqueryPath = path.join(__dirname, '/static/jquery-2.0.3.min.js'); - const src = ` `; - const encoded = btoa(unescape(encodeURIComponent(src))); - - const message = await startLoadingWebViewAndWaitForMessage(webview, { - disablewebsecurity: '', - src: `data:text/html;base64,${encoded}` - }); - expect(message).to.equal('ok'); + const result = await loadFileInWebView(webview, { disablewebsecurity: '' }); + expect(result).to.equal('loaded'); }); it('does not break node integration', async () => { @@ -483,16 +491,10 @@ describe(' tag', function () { }); it('can disables web security and enable nodeintegration', async () => { - const jqueryPath = path.join(__dirname, '/static/jquery-2.0.3.min.js'); - const src = ` `; - const encoded = btoa(unescape(encodeURIComponent(src))); - - const message = await startLoadingWebViewAndWaitForMessage(webview, { - src: `data:text/html;base64,${encoded}`, - webpreferences: 'webSecurity=no, nodeIntegration=yes, contextIsolation=no' - }); - - expect(message).to.equal('function'); + const result = await loadFileInWebView(webview, { webpreferences: 'webSecurity=no, nodeIntegration=yes, contextIsolation=no' }); + expect(result).to.equal('loaded'); + const type = await webview.executeJavaScript('typeof require'); + expect(type).to.equal('function'); }); });