From b965e54efc1db8117fe5e9131ce071d1ddc47ed9 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Tue, 22 Jan 2019 02:08:16 +0100 Subject: [PATCH] fix: not working with contextIsolation + sandbox (#16469) --- .../atom_sandboxed_renderer_client.cc | 24 +++++ .../renderer/atom_sandboxed_renderer_client.h | 2 +- filenames.gni | 1 + lib/isolated_renderer/init.js | 15 ++- lib/renderer/init.js | 54 ++++------ lib/renderer/web-view/web-view-init.js | 35 +++++++ lib/sandboxed_renderer/init.js | 17 ++-- spec/webview-spec.js | 98 +++++++++++++------ 8 files changed, 164 insertions(+), 82 deletions(-) create mode 100644 lib/renderer/web-view/web-view-init.js diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index dad1a667f0386..2744e9aadf26b 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.cc +++ b/atom/renderer/atom_sandboxed_renderer_client.cc @@ -202,6 +202,30 @@ void AtomSandboxedRendererClient::DidCreateScriptContext( &preload_bundle_params, &preload_bundle_args, nullptr); } +void AtomSandboxedRendererClient::SetupMainWorldOverrides( + v8::Handle context, + content::RenderFrame* render_frame) { + // Setup window overrides in the main world context + // Wrap the bundle into a function that receives the isolatedWorld as + // an argument. + auto* isolate = context->GetIsolate(); + + mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate); + process.SetMethod("binding", GetBinding); + + std::vector> isolated_bundle_params = { + node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"), + node::FIXED_ONE_BYTE_STRING(isolate, "isolatedWorld")}; + + std::vector> isolated_bundle_args = { + process.GetHandle(), + GetContext(render_frame->GetWebFrame(), isolate)->Global()}; + + node::per_process::native_module_loader.CompileAndCall( + context, "electron/js2c/isolated_bundle", &isolated_bundle_params, + &isolated_bundle_args, nullptr); +} + void AtomSandboxedRendererClient::WillReleaseScriptContext( v8::Handle context, content::RenderFrame* render_frame) { diff --git a/atom/renderer/atom_sandboxed_renderer_client.h b/atom/renderer/atom_sandboxed_renderer_client.h index 8c165d4b5be15..ac543fbe62017 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.h +++ b/atom/renderer/atom_sandboxed_renderer_client.h @@ -29,7 +29,7 @@ class AtomSandboxedRendererClient : public RendererClientBase { void WillReleaseScriptContext(v8::Handle context, content::RenderFrame* render_frame) override; void SetupMainWorldOverrides(v8::Handle context, - content::RenderFrame* render_frame) override {} + content::RenderFrame* render_frame) override; // content::ContentRendererClient: void RenderFrameCreated(content::RenderFrame*) override; void RenderViewCreated(content::RenderView*) override; diff --git a/filenames.gni b/filenames.gni index 4004edccf1334..921415a984a5f 100644 --- a/filenames.gni +++ b/filenames.gni @@ -74,6 +74,7 @@ filenames = { "lib/renderer/web-view/web-view-constants.js", "lib/renderer/web-view/web-view-element.js", "lib/renderer/web-view/web-view-impl.js", + "lib/renderer/web-view/web-view-init.js", "lib/renderer/api/exports/electron.js", "lib/renderer/api/crash-reporter.js", "lib/renderer/api/ipc-renderer.js", diff --git a/lib/isolated_renderer/init.js b/lib/isolated_renderer/init.js index 17dad47c1a245..e9b4c03f1f4eb 100644 --- a/lib/isolated_renderer/init.js +++ b/lib/isolated_renderer/init.js @@ -2,11 +2,11 @@ /* global nodeProcess, isolatedWorld */ -// Note: Don't use "process", as it will be replaced by browserify's one. -const v8Util = nodeProcess.atomBinding('v8_util') +const atomBinding = require('@electron/internal/common/atom-binding-setup')(nodeProcess.binding, 'renderer') -const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args') -const { webViewImpl, ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs +const v8Util = atomBinding('v8_util') + +const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl') if (webViewImpl) { // Must setup the WebView element in main world. @@ -14,4 +14,9 @@ if (webViewImpl) { setupWebView(v8Util, webViewImpl) } -require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen) +const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args') + +if (isolatedWorldArgs) { + const { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs + require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen) +} diff --git a/lib/renderer/init.js b/lib/renderer/init.js index a514fddf57be5..6462548fa162f 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -58,33 +58,29 @@ if (preloadScript) { preloadScripts.push(preloadScript) } -if (window.location.protocol === 'chrome-devtools:') { - // Override some inspector APIs. - require('@electron/internal/renderer/inspector') -} else if (window.location.protocol === 'chrome-extension:') { - // Add implementations of chrome API. - require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window) -} else if (window.location.protocol === 'chrome:') { - // Disable node integration for chrome UI scheme. -} else { - // Override default web functions. - require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen) - - // Inject content scripts. - require('@electron/internal/renderer/content-scripts-injector') +switch (window.location.protocol) { + case 'chrome-devtools:': { + // Override some inspector APIs. + require('@electron/internal/renderer/inspector') + break + } + case 'chrome-extension:': { + // Inject the chrome.* APIs that chrome extensions require + require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window) + break + } + default: { + // Override default web functions. + require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen) - // Load webview tag implementation. - if (webviewTag && guestInstanceId == null) { - const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl') - if (contextIsolation) { - Object.assign(isolatedWorldArgs, { webViewImpl }) - } else { - const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') - setupWebView(v8Util, webViewImpl) - } + // Inject content scripts. + require('@electron/internal/renderer/content-scripts-injector') } } +// Load webview tag implementation. +require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, webviewTag, guestInstanceId) + // Pass the arguments to isolatedWorld. if (contextIsolation) { v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs) @@ -163,15 +159,3 @@ for (const preloadScript of preloadScripts) { // Warn about security issues require('@electron/internal/renderer/security-warnings')(nodeIntegration) - -// Report focus/blur events of webview to browser. -// Note that while Chromium content APIs have observer for focus/blur, they -// unfortunately do not work for webview. -if (guestInstanceId) { - window.addEventListener('focus', () => { - ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId) - }) - window.addEventListener('blur', () => { - ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId) - }) -} diff --git a/lib/renderer/web-view/web-view-init.js b/lib/renderer/web-view/web-view-init.js new file mode 100644 index 0000000000000..6afff49c14b1f --- /dev/null +++ b/lib/renderer/web-view/web-view-init.js @@ -0,0 +1,35 @@ +'use strict' + +const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal') +const v8Util = process.atomBinding('v8_util') + +function handleFocusBlur (guestInstanceId) { + // Note that while Chromium content APIs have observer for focus/blur, they + // unfortunately do not work for webview. + + window.addEventListener('focus', () => { + ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId) + }) + + window.addEventListener('blur', () => { + ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId) + }) +} + +module.exports = function (contextIsolation, webviewTag, guestInstanceId) { + // Load webview tag implementation. + if (webviewTag && guestInstanceId == null) { + const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl') + if (contextIsolation) { + v8Util.setHiddenValue(window, 'web-view-impl', webViewImpl) + } else { + const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') + setupWebView(v8Util, webViewImpl) + } + } + + if (guestInstanceId) { + // Report focus/blur events of webview to browser. + handleFocusBlur(guestInstanceId) + } +} diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js index 0427816830069..f45e8edb35cce 100644 --- a/lib/sandboxed_renderer/init.js +++ b/lib/sandboxed_renderer/init.js @@ -105,8 +105,12 @@ function preloadRequire (module) { throw new Error('module not found') } +// Process command line arguments. const { hasSwitch } = process.atomBinding('command_line') +const isBackgroundPage = hasSwitch('background-page') +const contextIsolation = hasSwitch('context-isolation') + switch (window.location.protocol) { case 'chrome-devtools:': { // Override some inspector APIs. @@ -115,22 +119,15 @@ switch (window.location.protocol) { } case 'chrome-extension:': { // Inject the chrome.* APIs that chrome extensions require - const isBackgroundPage = hasSwitch('background-page') require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window) break } } -if (binding.guestInstanceId) { - process.guestInstanceId = parseInt(binding.guestInstanceId) -} +const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId) -// Don't allow recursive ``. -if (!process.guestInstanceId && isWebViewTagEnabled) { - const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl') - const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') - setupWebView(v8Util, webViewImpl) -} +// Load webview tag implementation. +require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, isWebViewTagEnabled, guestInstanceId) const errorUtils = require('@electron/internal/common/error-utils') diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 9370a01bb6da7..12e179143fa2d 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -76,6 +76,19 @@ describe(' tag', function () { await emittedOnce(ipcMain, 'pong') }) + it('works with sandbox', async () => { + const w = await openTheWindow({ + show: false, + webPreferences: { + webviewTag: true, + nodeIntegration: true, + sandbox: true + } + }) + w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html')) + await emittedOnce(ipcMain, 'pong') + }) + it('works with contextIsolation', async () => { const w = await openTheWindow({ show: false, @@ -89,6 +102,20 @@ describe(' tag', function () { await emittedOnce(ipcMain, 'pong') }) + it('works with contextIsolation + sandbox', async () => { + const w = await openTheWindow({ + show: false, + webPreferences: { + webviewTag: true, + nodeIntegration: true, + contextIsolation: true, + sandbox: true + } + }) + w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html')) + await emittedOnce(ipcMain, 'pong') + }) + it('is disabled by default', async () => { const w = await openTheWindow({ show: false, @@ -1349,47 +1376,56 @@ describe(' tag', function () { if (div != null) div.remove() }) - it('emits resize events', async () => { - const firstResizeSignal = waitForEvent(webview, 'resize') - const domReadySignal = waitForEvent(webview, 'dom-ready') + const generateSpecs = (description, sandbox) => { + describe(description, () => { + it('emits resize events', async () => { + const firstResizeSignal = waitForEvent(webview, 'resize') + const domReadySignal = waitForEvent(webview, 'dom-ready') - webview.src = `file://${fixtures}/pages/a.html` - div.appendChild(webview) - document.body.appendChild(div) + webview.src = `file://${fixtures}/pages/a.html` + webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}` + div.appendChild(webview) + document.body.appendChild(div) - const firstResizeEvent = await firstResizeSignal - expect(firstResizeEvent.target).to.equal(webview) - expect(firstResizeEvent.newWidth).to.equal(100) - expect(firstResizeEvent.newHeight).to.equal(10) + const firstResizeEvent = await firstResizeSignal + expect(firstResizeEvent.target).to.equal(webview) + expect(firstResizeEvent.newWidth).to.equal(100) + expect(firstResizeEvent.newHeight).to.equal(10) - await domReadySignal + await domReadySignal - const secondResizeSignal = waitForEvent(webview, 'resize') + const secondResizeSignal = waitForEvent(webview, 'resize') - const newWidth = 1234 - const newHeight = 789 - div.style.width = `${newWidth}px` - div.style.height = `${newHeight}px` + const newWidth = 1234 + const newHeight = 789 + div.style.width = `${newWidth}px` + div.style.height = `${newHeight}px` - const secondResizeEvent = await secondResizeSignal - expect(secondResizeEvent.target).to.equal(webview) - expect(secondResizeEvent.newWidth).to.equal(newWidth) - expect(secondResizeEvent.newHeight).to.equal(newHeight) - }) + const secondResizeEvent = await secondResizeSignal + expect(secondResizeEvent.target).to.equal(webview) + expect(secondResizeEvent.newWidth).to.equal(newWidth) + expect(secondResizeEvent.newHeight).to.equal(newHeight) + }) - it('emits focus event', async () => { - const domReadySignal = waitForEvent(webview, 'dom-ready') - webview.src = `file://${fixtures}/pages/a.html` - document.body.appendChild(webview) + it('emits focus event', async () => { + const domReadySignal = waitForEvent(webview, 'dom-ready') + webview.src = `file://${fixtures}/pages/a.html` + webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}` + document.body.appendChild(webview) - await domReadySignal + await domReadySignal - // If this test fails, check if webview.focus() still works. - const focusSignal = waitForEvent(webview, 'focus') - webview.focus() + // If this test fails, check if webview.focus() still works. + const focusSignal = waitForEvent(webview, 'focus') + webview.focus() - await focusSignal - }) + await focusSignal + }) + }) + } + + generateSpecs('without sandbox', false) + generateSpecs('with sandbox', true) }) describe('zoom behavior', () => {