From be6fb7cb12687b26730b9b2672f01c30889e6ced Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 16 Apr 2019 13:57:02 -0400 Subject: [PATCH] fix: ensure the sandboxed preloads globals do not leak (#17712) --- BUILD.gn | 61 +++++++++++++++++++++++++++++---- build/js_wrap.gni | 19 ++++++++++ build/js_wrap.py | 19 ++++++++++ spec/api-browser-window-spec.js | 42 +++++++++++++++++++++++ spec/fixtures/api/no-leak.html | 17 +++++++++ spec/fixtures/module/empty.js | 5 +++ 6 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 build/js_wrap.gni create mode 100644 build/js_wrap.py create mode 100644 spec/fixtures/api/no-leak.html create mode 100644 spec/fixtures/module/empty.js diff --git a/BUILD.gn b/BUILD.gn index c625af113c3f..6d09a61c16e3 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -10,6 +10,7 @@ import("//tools/grit/repack.gni") import("//tools/v8_context_snapshot/v8_context_snapshot.gni") import("//v8/snapshot_toolchain.gni") import("build/asar.gni") +import("build/js_wrap.gni") import("build/npm.gni") import("build/tsc.gni") import("buildflags/buildflags.gni") @@ -70,7 +71,7 @@ npm_action("build_electron_definitions") { ] } -npm_action("atom_browserify_sandbox") { +npm_action("atom_browserify_sandbox_unwrapped") { script = "browserify" deps = [ ":build_electron_definitions", @@ -79,7 +80,7 @@ npm_action("atom_browserify_sandbox") { inputs = auto_filenames.sandbox_browserify_deps outputs = [ - "$target_gen_dir/js2c/preload_bundle.js", + "$target_gen_dir/js2c/preload_bundle_unwrapped.js", ] args = [ @@ -94,12 +95,14 @@ npm_action("atom_browserify_sandbox") { "-p", "tsconfig.electron.json", "]", + "--standalone", + "sandboxed_preload", "-o", rebase_path(outputs[0]), ] } -npm_action("atom_browserify_isolated") { +npm_action("atom_browserify_isolated_unwrapped") { script = "browserify" deps = [ ":build_electron_definitions", @@ -108,7 +111,7 @@ npm_action("atom_browserify_isolated") { inputs = auto_filenames.isolated_browserify_deps outputs = [ - "$target_gen_dir/js2c/isolated_bundle.js", + "$target_gen_dir/js2c/isolated_bundle_unwrapped.js", ] args = [ @@ -121,12 +124,14 @@ npm_action("atom_browserify_isolated") { "-p", "tsconfig.electron.json", "]", + "--standalone", + "isolated_preload", "-o", rebase_path(outputs[0]), ] } -npm_action("atom_browserify_content_script") { +npm_action("atom_browserify_content_script_unwrapped") { script = "browserify" deps = [ ":build_electron_definitions", @@ -135,7 +140,7 @@ npm_action("atom_browserify_content_script") { inputs = auto_filenames.context_script_browserify_deps outputs = [ - "$target_gen_dir/js2c/content_script_bundle.js", + "$target_gen_dir/js2c/content_script_bundle_unwrapped.js", ] args = [ @@ -148,11 +153,55 @@ npm_action("atom_browserify_content_script") { "-p", "tsconfig.electron.json", "]", + "--standalone", + "content_script_preload", "-o", rebase_path(outputs[0]), ] } +js_wrap("atom_browserify_content_script") { + deps = [ + ":atom_browserify_content_script_unwrapped", + ] + + inputs = [ + "$target_gen_dir/js2c/content_script_bundle_unwrapped.js", + ] + + outputs = [ + "$target_gen_dir/js2c/content_script_bundle.js", + ] +} + +js_wrap("atom_browserify_isolated") { + deps = [ + ":atom_browserify_isolated_unwrapped", + ] + + inputs = [ + "$target_gen_dir/js2c/isolated_bundle_unwrapped.js", + ] + + outputs = [ + "$target_gen_dir/js2c/isolated_bundle.js", + ] +} + +js_wrap("atom_browserify_sandbox") { + deps = [ + ":atom_browserify_sandbox_unwrapped", + ] + + inputs = [ + "$target_gen_dir/js2c/preload_bundle_unwrapped.js", + ] + + outputs = [ + "$target_gen_dir/js2c/preload_bundle.js", + ] +} + copy("atom_js2c_copy") { sources = [ "lib/common/asar.js", diff --git a/build/js_wrap.gni b/build/js_wrap.gni new file mode 100644 index 000000000000..d12b4bfae7be --- /dev/null +++ b/build/js_wrap.gni @@ -0,0 +1,19 @@ +template("js_wrap") { + assert(defined(invoker.inputs), "Need input JS script") + assert(defined(invoker.outputs), "Need output JS script") + + action(target_name) { + forward_variables_from(invoker, + [ + "deps", + "public_deps", + "sources", + "inputs", + "outputs", + ]) + + script = "//electron/build/js_wrap.py" + args = [ "--in" ] + rebase_path(invoker.inputs) + [ "--out" ] + + rebase_path(invoker.outputs) + } +} diff --git a/build/js_wrap.py b/build/js_wrap.py new file mode 100644 index 000000000000..d77f80f788b7 --- /dev/null +++ b/build/js_wrap.py @@ -0,0 +1,19 @@ +import sys + +in_start = sys.argv.index("--in") + 1 +out_start = sys.argv.index("--out") + 1 + +in_bundles = sys.argv[in_start:out_start - 1] +out_bundles = sys.argv[out_start:] + +if len(in_bundles) is not len(out_bundles): + print("--out and --in must provide the same number of arguments") + sys.exit(1) + +for i in range(len(in_bundles)): + in_bundle = in_bundles[i] + out_path = out_bundles[i] + with open(in_bundle, 'r') as f: + lines = ["(function(){var exports={},module={exports};"] + f.readlines() + ["})();"] + with open(out_path, 'w') as out_f: + out_f.writelines(lines) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 0f0322d5447b..9f746ad6d9a7 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1357,6 +1357,48 @@ describe('BrowserWindow module', () => { afterEach(() => { ipcMain.removeAllListeners('answer') }) describe('"preload" option', () => { + const doesNotLeakSpec = (name, webPrefs) => { + it(name, async function () { + w.destroy() + w = new BrowserWindow({ + webPreferences: { + ...webPrefs, + preload: path.resolve(fixtures, 'module', 'empty.js') + }, + show: false + }) + const leakResult = emittedOnce(ipcMain, 'leak-result') + w.loadFile(path.join(fixtures, 'api', 'no-leak.html')) + const [, result] = await leakResult + console.log(result) + expect(result).to.have.property('require', 'undefined') + expect(result).to.have.property('exports', 'undefined') + expect(result).to.have.property('windowExports', 'undefined') + expect(result).to.have.property('windowPreload', 'undefined') + expect(result).to.have.property('windowRequire', 'undefined') + }) + } + doesNotLeakSpec('does not leak require', { + nodeIntegration: false, + sandbox: false, + contextIsolation: false + }) + doesNotLeakSpec('does not leak require when sandbox is enabled', { + nodeIntegration: false, + sandbox: true, + contextIsolation: false + }) + doesNotLeakSpec('does not leak require when context isolation is enabled', { + nodeIntegration: false, + sandbox: false, + contextIsolation: true + }) + doesNotLeakSpec('does not leak require when context isolation and sandbox are enabled', { + nodeIntegration: false, + sandbox: true, + contextIsolation: true + }) + it('loads the script before other scripts in window', async () => { const preload = path.join(fixtures, 'module', 'set-global.js') w.destroy() diff --git a/spec/fixtures/api/no-leak.html b/spec/fixtures/api/no-leak.html new file mode 100644 index 000000000000..22201dc9ca0c --- /dev/null +++ b/spec/fixtures/api/no-leak.html @@ -0,0 +1,17 @@ + + + + Document + + + + + \ No newline at end of file diff --git a/spec/fixtures/module/empty.js b/spec/fixtures/module/empty.js new file mode 100644 index 000000000000..3f0e6f4f0707 --- /dev/null +++ b/spec/fixtures/module/empty.js @@ -0,0 +1,5 @@ +const { ipcRenderer } = require('electron') + +window.addEventListener('message', (event) => { + ipcRenderer.send('leak-result', event.data) +})