From 2713446a5142ccb7dd5348833f6b737c4021778e Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Thu, 9 Nov 2023 10:23:52 -0800 Subject: [PATCH] feat: add new fuse to treat file: identically to browsers (#40372) --- build/fuses/fuses.json5 | 3 ++- docs/api/protocol.md | 16 +++++++++++++--- docs/tutorial/fuses.md | 13 +++++++++++++ docs/tutorial/security.md | 21 +++++++++++++++++++++ shell/app/electron_content_client.cc | 5 ++++- shell/browser/electron_browser_client.cc | 7 +++++-- shell/renderer/renderer_client_base.cc | 7 +++++-- 7 files changed, 63 insertions(+), 9 deletions(-) diff --git a/build/fuses/fuses.json5 b/build/fuses/fuses.json5 index e8df5ffd7ab98..3b5916ec7e1fb 100644 --- a/build/fuses/fuses.json5 +++ b/build/fuses/fuses.json5 @@ -8,5 +8,6 @@ "node_cli_inspect": "1", "embedded_asar_integrity_validation": "0", "only_load_app_from_asar": "0", - "load_browser_process_specific_v8_snapshot": "0" + "load_browser_process_specific_v8_snapshot": "0", + "grant_file_protocol_extra_privileges": "1" } diff --git a/docs/api/protocol.md b/docs/api/protocol.md index 10a1c71cdf66e..93dc050287bdf 100644 --- a/docs/api/protocol.md +++ b/docs/api/protocol.md @@ -122,7 +122,7 @@ Example: ```js const { app, net, protocol } = require('electron') -const { join } = require('node:path') +const path = require('node:path') const { pathToFileURL } = require('url') protocol.registerSchemesAsPrivileged([ @@ -145,9 +145,19 @@ app.whenReady().then(() => { headers: { 'content-type': 'text/html' } }) } - // NB, this does not check for paths that escape the bundle, e.g. + // NB, this checks for paths that escape the bundle, e.g. // app://bundle/../../secret_file.txt - return net.fetch(pathToFileURL(join(__dirname, pathname)).toString()) + const pathToServe = path.resolve(__dirname, pathname) + const relativePath = path.relative(__dirname, pathToServe) + const isSafe = relativePath && !relativePath.startsWith('..') && !path.isAbsolute(relativePath) + if (!isSafe) { + return new Response('bad', { + status: 400, + headers: { 'content-type': 'text/html' } + }) + } + + return net.fetch(pathToFileURL(pathToServe).toString()) } else if (host === 'api') { return net.fetch('https://api.my-server.com/' + pathname, { method: req.method, diff --git a/docs/tutorial/fuses.md b/docs/tutorial/fuses.md index ea5b8a3428d17..4269b2bf3602f 100644 --- a/docs/tutorial/fuses.md +++ b/docs/tutorial/fuses.md @@ -61,6 +61,19 @@ The onlyLoadAppFromAsar fuse changes the search system that Electron uses to loc The loadBrowserProcessSpecificV8Snapshot fuse changes which V8 snapshot file is used for the browser process. By default Electron's processes will all use the same V8 snapshot file. When this fuse is enabled the browser process uses the file called `browser_v8_context_snapshot.bin` for its V8 snapshot. The other processes will use the V8 snapshot file that they normally do. +### `grantFileProtocolExtraPrivileges` + +**Default:** Enabled +**@electron/fuses:** `FuseV1Options.GrantFileProtocolExtraPrivileges` + +The grantFileProtocolExtraPrivileges fuse changes whether pages loaded from the `file://` protocol are given privileges beyond what they would receive in a traditional web browser. This behavior was core to Electron apps in original versions of Electron but is no longer required as apps should be [serving local files from custom protocols](./security.md#18-avoid-usage-of-the-file-protocol-and-prefer-usage-of-custom-protocols) now instead. If you aren't serving pages from `file://` you should disable this fuse. + +The extra privileges granted to the `file://` protocol by this fuse are incompletely documented below: + +* `file://` protocol pages can use `fetch` to load other assets over `file://` +* `file://` protocol pages can use service workers +* `file://` protocol pages have universal access granted to child frames also running on `file://` protocols regardless of sandbox settings + ## How do I flip the fuses? ### The easy way diff --git a/docs/tutorial/security.md b/docs/tutorial/security.md index e277c722ae56d..1da0f702cc955 100644 --- a/docs/tutorial/security.md +++ b/docs/tutorial/security.md @@ -759,6 +759,27 @@ function validateSender (frame) { } ``` +### 18. Avoid usage of the `file://` protocol and prefer usage of custom protocols + +You should serve local pages from a custom protocol instead of the `file://` protocol. + +#### Why? + +The `file://` protocol gets more privileges in Electron than in a web browser and even in +browsers it is treated differently to http/https URLs. Using a custom protocol allows you +to be more aligned with classic web url behavior while retaining even more control about +what can be loaded and when. + +Pages running on `file://` have unilateral access to every file on your machine meaning +that XSS issues can be used to load arbitrary files from the users machine. Using a custom +protocol prevents issues like this as you can limit the protocol to only serving a specific +set of files. + +#### How? + +Follow the [`protocol.handle`](../api/protocol.md#protocolhandlescheme-handler) examples to +learn how to serve files / content from a custom protocol. + [breaking-changes]: ../breaking-changes.md [browser-window]: ../api/browser-window.md [browser-view]: ../api/browser-view.md diff --git a/shell/app/electron_content_client.cc b/shell/app/electron_content_client.cc index 9a37ac4149f47..7e6da642bb73b 100644 --- a/shell/app/electron_content_client.cc +++ b/shell/app/electron_content_client.cc @@ -17,6 +17,7 @@ #include "content/public/common/content_constants.h" #include "content/public/common/content_switches.h" #include "electron/buildflags/buildflags.h" +#include "electron/fuses.h" #include "extensions/common/constants.h" #include "pdf/buildflags.h" #include "ppapi/buildflags/buildflags.h" @@ -168,7 +169,9 @@ void ElectronContentClient::AddAdditionalSchemes(Schemes* schemes) { &schemes->cors_enabled_schemes); } - schemes->service_worker_schemes.emplace_back(url::kFileScheme); + if (electron::fuses::IsGrantFileProtocolExtraPrivilegesEnabled()) { + schemes->service_worker_schemes.emplace_back(url::kFileScheme); + } #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) schemes->standard_schemes.push_back(extensions::kExtensionScheme); diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index cdac20a9af996..3414cd40c27b9 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -56,6 +56,7 @@ #include "content/public/common/url_constants.h" #include "crypto/crypto_buildflags.h" #include "electron/buildflags/buildflags.h" +#include "electron/fuses.h" #include "electron/shell/common/api/api.mojom.h" #include "extensions/browser/extension_navigation_ui_data.h" #include "mojo/public/cpp/bindings/binder_map.h" @@ -425,8 +426,10 @@ void ElectronBrowserClient::OverrideWebkitPrefs( prefs->javascript_can_access_clipboard = true; prefs->local_storage_enabled = true; prefs->databases_enabled = true; - prefs->allow_universal_access_from_file_urls = true; - prefs->allow_file_access_from_file_urls = true; + prefs->allow_universal_access_from_file_urls = + electron::fuses::IsGrantFileProtocolExtraPrivilegesEnabled(); + prefs->allow_file_access_from_file_urls = + electron::fuses::IsGrantFileProtocolExtraPrivilegesEnabled(); prefs->webgl1_enabled = true; prefs->webgl2_enabled = true; prefs->allow_running_insecure_content = false; diff --git a/shell/renderer/renderer_client_base.cc b/shell/renderer/renderer_client_base.cc index 11970869b43aa..6bf39c06ea283 100644 --- a/shell/renderer/renderer_client_base.cc +++ b/shell/renderer/renderer_client_base.cc @@ -19,6 +19,7 @@ #include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_thread.h" #include "electron/buildflags/buildflags.h" +#include "electron/fuses.h" #include "printing/buildflags/buildflags.h" #include "shell/browser/api/electron_api_protocol.h" #include "shell/common/api/electron_api_native_image.h" @@ -277,8 +278,10 @@ void RendererClientBase::RenderThreadStarted() { // Allow file scheme to handle service worker by default. // FIXME(zcbenz): Can this be moved elsewhere? - blink::WebSecurityPolicy::RegisterURLSchemeAsAllowingServiceWorkers("file"); - blink::SchemeRegistry::RegisterURLSchemeAsSupportingFetchAPI("file"); + if (electron::fuses::IsGrantFileProtocolExtraPrivilegesEnabled()) { + blink::WebSecurityPolicy::RegisterURLSchemeAsAllowingServiceWorkers("file"); + blink::SchemeRegistry::RegisterURLSchemeAsSupportingFetchAPI("file"); + } #if BUILDFLAG(IS_WIN) // Set ApplicationUserModelID in renderer process.