From 4575a4aae39ac266181da0d66da75fae9bede400 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Thu, 20 Jun 2019 01:39:12 +0200 Subject: [PATCH] Revert "feat: only allow bundled preload scripts (#17308)" (#18091) This reverts commit 8cf15cc9310ca5983f15e511c3ecdac8e75e225d. --- docs/api/breaking-changes.md | 4 ---- docs/api/browser-window.md | 2 -- docs/api/session.md | 2 -- docs/api/webview-tag.md | 3 --- filenames.auto.gni | 2 -- lib/browser/rpc-server.js | 13 ------------- lib/common/path-utils.ts | 6 ------ lib/renderer/init.ts | 13 ------------- spec/api-web-contents-spec.js | 31 ------------------------------- 9 files changed, 76 deletions(-) delete mode 100644 lib/common/path-utils.ts diff --git a/docs/api/breaking-changes.md b/docs/api/breaking-changes.md index 047f103283bc..bc1ac327f8b1 100644 --- a/docs/api/breaking-changes.md +++ b/docs/api/breaking-changes.md @@ -148,10 +148,6 @@ app.enableMixedSandbox() Mixed-sandbox mode is now enabled by default. -### Preload scripts outside of app path are not allowed - -For security reasons, preload scripts can only be loaded from a subpath of the [app path](app.md#appgetapppath). - ## Planned Breaking API Changes (5.0) ### `new BrowserWindow({ webPreferences })` diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 4c16ea6ca32b..71dceea8eafb 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -266,8 +266,6 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. When node integration is turned off, the preload script can reintroduce Node global symbols back to the global scope. See example [here](process.md#event-loaded). - **Note:** For security reasons, preload scripts can only be loaded from - a subpath of the [app path](app.md#appgetapppath). * `sandbox` Boolean (optional) - If set, this will sandbox the renderer associated with the window, making it compatible with the Chromium OS-level sandbox and disabling the Node.js engine. This is not the same as diff --git a/docs/api/session.md b/docs/api/session.md index 1b0116c11833..a38737d39d1c 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -418,8 +418,6 @@ Returns `Promise` - resolves when the session’s HTTP authentication cach Adds scripts that will be executed on ALL web contents that are associated with this session just before normal `preload` scripts run. -**Note:** For security reasons, preload scripts can only be loaded from a subpath of the [app path](app.md#appgetapppath). - #### `ses.getPreloads()` Returns `String[]` an array of paths to preload scripts that have been diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index cb2921d38465..84d9ff0ad582 100644 --- a/docs/api/webview-tag.md +++ b/docs/api/webview-tag.md @@ -162,9 +162,6 @@ When the guest page doesn't have node integration this script will still have access to all Node APIs, but global objects injected by Node will be deleted after this script has finished executing. -**Note:** For security reasons, preload scripts can only be loaded from -a subpath of the [app path](app.md#appgetapppath). - **Note:** This option will be appear as `preloadURL` (not `preload`) in the `webPreferences` specified to the `will-attach-webview` event. diff --git a/filenames.auto.gni b/filenames.auto.gni index fd74881e17f1..6f71eb59761d 100644 --- a/filenames.auto.gni +++ b/filenames.auto.gni @@ -256,7 +256,6 @@ auto_filenames = { "lib/common/error-utils.js", "lib/common/init.ts", "lib/common/parse-features-string.js", - "lib/common/path-utils.ts", "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.js", "lib/renderer/ipc-renderer-internal-utils.ts", @@ -281,7 +280,6 @@ auto_filenames = { "lib/common/electron-binding-setup.ts", "lib/common/error-utils.js", "lib/common/init.ts", - "lib/common/path-utils.ts", "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.js", "lib/renderer/api/crash-reporter.js", diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 17d21436440d..af2f13ce23ca 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -3,7 +3,6 @@ const electron = require('electron') const { EventEmitter } = require('events') const fs = require('fs') -const path = require('path') const v8Util = process.electronBinding('v8_util') const eventBinding = process.electronBinding('event') @@ -21,7 +20,6 @@ const guestViewManager = require('@electron/internal/browser/guest-view-manager' const bufferUtils = require('@electron/internal/common/buffer-utils') const errorUtils = require('@electron/internal/common/error-utils') const clipboardUtils = require('@electron/internal/common/clipboard-utils') -const { isParentDir } = require('@electron/internal/common/path-utils') const hasProp = {}.hasOwnProperty @@ -518,22 +516,11 @@ if (features.isDesktopCapturerEnabled()) { }) } -let absoluteAppPath -const getAppPath = async function () { - if (absoluteAppPath === undefined) { - absoluteAppPath = await fs.promises.realpath(electron.app.getAppPath()) - } - return absoluteAppPath -} - const getPreloadScript = async function (preloadPath) { let preloadSrc = null let preloadError = null if (preloadPath) { try { - if (!isParentDir(await getAppPath(), await fs.promises.realpath(preloadPath))) { - throw new Error('Preload scripts outside of app path are not allowed') - } preloadSrc = (await fs.promises.readFile(preloadPath)).toString() } catch (err) { preloadError = errorUtils.serialize(err) diff --git a/lib/common/path-utils.ts b/lib/common/path-utils.ts deleted file mode 100644 index a55e99941082..000000000000 --- a/lib/common/path-utils.ts +++ /dev/null @@ -1,6 +0,0 @@ -import * as path from 'path' - -export const isParentDir = function (parent: string, dir: string) { - const relative = path.relative(parent, dir) - return !!relative && !relative.startsWith('..') && !path.isAbsolute(relative) -} diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 4622695cd81a..254020d2e5dd 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -1,5 +1,4 @@ import { EventEmitter } from 'events' -import * as fs from 'fs' import * as path from 'path' const Module = require('module') @@ -191,22 +190,10 @@ if (nodeIntegration) { } const errorUtils = require('@electron/internal/common/error-utils') -const { isParentDir } = require('@electron/internal/common/path-utils') - -let absoluteAppPath: string -const getAppPath = function () { - if (absoluteAppPath === undefined) { - absoluteAppPath = fs.realpathSync(appPath!) - } - return absoluteAppPath -} // Load the preload scripts. for (const preloadScript of preloadScripts) { try { - if (!isParentDir(getAppPath(), fs.realpathSync(preloadScript))) { - throw new Error('Preload scripts outside of app path are not allowed') - } Module._load(preloadScript) } catch (error) { console.error(`Unable to load preload script: ${preloadScript}`) diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index 203704e0e541..aed83a59bf06 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -2,7 +2,6 @@ const ChildProcess = require('child_process') const fs = require('fs') -const os = require('os') const http = require('http') const path = require('path') const { closeWindow } = require('./window-helpers') @@ -1072,16 +1071,6 @@ describe('webContents module', () => { describe('preload-error event', () => { const generateSpecs = (description, sandbox) => { describe(description, () => { - const tmpPreload = path.join(os.tmpdir(), 'preload.js') - - before((done) => { - fs.writeFile(tmpPreload, '', done) - }) - - after((done) => { - fs.unlink(tmpPreload, () => done()) - }) - it('is triggered when unhandled exception is thrown', async () => { const preload = path.join(fixtures, 'module', 'preload-error-exception.js') @@ -1141,26 +1130,6 @@ describe('webContents module', () => { expect(preloadPath).to.equal(preload) expect(error.message).to.contain('preload-invalid.js') }) - - it('is triggered when preload script is outside of app path', async () => { - const preload = tmpPreload - - w.destroy() - w = new BrowserWindow({ - show: false, - webPreferences: { - sandbox, - preload - } - }) - - const promise = emittedOnce(w.webContents, 'preload-error') - w.loadURL('about:blank') - - const [, preloadPath, error] = await promise - expect(preloadPath).to.equal(preload) - expect(error.message).to.contain('Preload scripts outside of app path are not allowed') - }) }) }