Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make grant_file_protocol_extra_privileges fuse also block CORS fetches #40864

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 19 additions & 1 deletion build/fuses/build.py
Expand Up @@ -32,6 +32,13 @@

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 {

Expand Down Expand Up @@ -66,9 +73,20 @@
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:]
Expand Down
28 changes: 16 additions & 12 deletions shell/browser/protocol_registry.cc
Expand Up @@ -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"

Expand All @@ -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_) {
Expand Down
5 changes: 4 additions & 1 deletion 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
Expand Down
28 changes: 28 additions & 0 deletions 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');
});
});