Browse Source

fix: allow accessing file:// when web security is disabled (#28560)

* fix: allow accessing file:// when web security is disabled

* fixup lint after merge

Co-authored-by: John Kleinschmidt <[email protected]>
Cheng Zhao 4 years ago
parent
commit
274f6f75b5

+ 22 - 10
shell/browser/electron_browser_client.cc

@@ -1282,9 +1282,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::
@@ -1293,8 +1294,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)
@@ -1365,9 +1368,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,
@@ -1375,10 +1391,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;
 

+ 13 - 14
shell/browser/protocol_registry.cc

@@ -6,6 +6,7 @@
 #include <utility>
 
 #include "content/public/browser/non_network_url_loader_factory_base.h"
+#include "content/public/browser/web_contents.h"
 #include "shell/browser/electron_browser_context.h"
 #include "shell/browser/net/asar/asar_url_loader.h"
 #include "shell/browser/protocol_registry.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_) {

+ 2 - 2
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_; }
 

+ 33 - 0
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,<script>
+          function loadFile() {
+            return new Promise((resolve) => {
+              fetch('${thisFile}').then(
+                () => resolve('loaded'),
+                () => resolve('failed')
+              )
+            });
+          }
+        </script>`);
+      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');
+    });
+  });
+
   describe('wasm-eval csp', () => {
     async function loadWasm (csp: string) {
       const w = new BrowserWindow({

+ 30 - 28
spec/webview-spec.js

@@ -33,6 +33,28 @@ describe('<webview> tag', function () {
     return event.message;
   };
 
+  async function loadFileInWebView (webview, attributes = {}) {
+    const thisFile = url.format({
+      pathname: __filename.replace(/\\/g, '/'),
+      protocol: 'file',
+      slashes: true
+    });
+    const src = `<script>
+      function loadFile() {
+        return new Promise((resolve) => {
+          fetch('${thisFile}').then(
+            () => resolve('loaded'),
+            () => resolve('failed')
+          )
+        });
+      }
+      console.log('ok');
+    </script>`;
+    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('<webview> 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 = `<script src='file://${jqueryPath}'></script> <script>console.log('ok');</script>`;
-      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 = `<script src='file://${jqueryPath}'></script> <script>console.log('ok');</script>`;
-      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 () => {
@@ -494,16 +502,10 @@ describe('<webview> 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 = `<script src='file://${jqueryPath}'></script> <script>console.log(typeof require);</script>`;
-      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');
     });
   });