Browse Source

fix: make grant_file_protocol_extra_privileges fuse also block CORS fetches (#40801)

Jeremy Rose 1 year ago
parent
commit
be4e4ff11b

+ 19 - 1
build/fuses/build.py

@@ -32,6 +32,13 @@ extern const volatile char kFuseWire[];
 
 TEMPLATE_CC = """
 #include "electron/fuses.h"
+#include "base/dcheck_is_on.h"
+
+#if DCHECK_IS_ON()
+#include "base/command_line.h"
+#include "base/strings/string_util.h"
+#include <string>
+#endif
 
 namespace electron::fuses {
 
@@ -66,9 +73,20 @@ for fuse in fuses:
   getters_h += "FUSE_EXPORT bool Is{name}Enabled();\n".replace("{name}", name)
   getters_cc += """
 bool Is{name}Enabled() {
+#if DCHECK_IS_ON()
+  // RunAsNode is checked so early that base::CommandLine isn't yet
+  // initialized, so guard here to avoid a CHECK.
+  if (base::CommandLine::InitializedForCurrentProcess()) {
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    if (command_line->HasSwitch("{switch_name}")) {
+      std::string switch_value = command_line->GetSwitchValueASCII("{switch_name}");
+      return switch_value == "1";
+    }
+  }
+#endif
   return kFuseWire[{index}] == '1';
 }
-""".replace("{name}", name).replace("{index}", str(index))
+""".replace("{name}", name).replace("{switch_name}", f"set-fuse-{fuse.lower()}").replace("{index}", str(index))
 
 def c_hex(n):
   s = hex(n)[2:]

+ 16 - 12
shell/browser/protocol_registry.cc

@@ -6,6 +6,7 @@
 
 #include "base/stl_util.h"
 #include "content/public/browser/web_contents.h"
+#include "electron/fuses.h"
 #include "shell/browser/electron_browser_context.h"
 #include "shell/browser/net/asar/asar_url_loader_factory.h"
 
@@ -24,18 +25,21 @@ ProtocolRegistry::~ProtocolRegistry() = default;
 void ProtocolRegistry::RegisterURLLoaderFactories(
     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 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());
+  if (electron::fuses::IsGrantFileProtocolExtraPrivilegesEnabled()) {
+    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 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());
+    }
   }
 
   for (const auto& it : handlers_) {

+ 4 - 1
spec/fixtures/apps/remote-control/main.js

@@ -1,4 +1,7 @@
-const { app } = require('electron');
+// eslint-disable-next-line camelcase
+const electron_1 = require('electron');
+// eslint-disable-next-line camelcase
+const { app } = electron_1;
 const http = require('node:http');
 const v8 = require('node:v8');
 // eslint-disable-next-line camelcase,@typescript-eslint/no-unused-vars

+ 28 - 0
spec/fuses-spec.ts

@@ -0,0 +1,28 @@
+import { expect } from 'chai';
+import { startRemoteControlApp } from './lib/spec-helpers';
+import { once } from 'node:events';
+import { spawn } from 'node:child_process';
+import { BrowserWindow } from 'electron';
+import path = require('node:path');
+
+describe('fuses', () => {
+  it('can be enabled by command-line argument during testing', async () => {
+    const child0 = spawn(process.execPath, ['-v'], { env: { NODE_OPTIONS: '-e 0' } });
+    const [code0] = await once(child0, 'exit');
+    // Should exit with 9 because -e is not allowed in NODE_OPTIONS
+    expect(code0).to.equal(9);
+    const child1 = spawn(process.execPath, ['--set-fuse-node_options=0', '-v'], { env: { NODE_OPTIONS: '-e 0' } });
+    const [code1] = await once(child1, 'exit');
+    // Should print the version and exit with 0
+    expect(code1).to.equal(0);
+  });
+
+  it('disables fetching file:// URLs when grant_file_protocol_extra_privileges is 0', async () => {
+    const rc = await startRemoteControlApp(['--set-fuse-grant_file_protocol_extra_privileges=0']);
+    await expect(rc.remotely(async (fixture: string) => {
+      const bw = new BrowserWindow({ show: false });
+      await bw.loadFile(fixture);
+      return await bw.webContents.executeJavaScript("ajax('file:///etc/passwd')");
+    }, path.join(__dirname, 'fixtures', 'pages', 'fetch.html'))).to.eventually.be.rejectedWith('Failed to fetch');
+  });
+});