From 8ec304f32f9d22997074cb7c8240bfa7d66e6a88 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Thu, 7 Mar 2019 17:46:57 -0500 Subject: [PATCH] fix: run subframe preload bundles in isolated context (#17165) * fix: run subframe preload bundles in isolated context * test subframe contextIsolation when disabled --- atom/renderer/atom_render_frame_observer.cc | 23 +++++++++-- spec/api-subframe-spec.js | 43 +++++++++++++++++---- spec/fixtures/sub-frames/preload.js | 2 + 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/atom/renderer/atom_render_frame_observer.cc b/atom/renderer/atom_render_frame_observer.cc index 4be1fb8536..6bddad4d08 100644 --- a/atom/renderer/atom_render_frame_observer.cc +++ b/atom/renderer/atom_render_frame_observer.cc @@ -12,6 +12,8 @@ #include "atom/common/heap_snapshot.h" #include "atom/common/native_mate_converters/value_converter.h" #include "atom/common/node_includes.h" +#include "atom/common/options_switches.h" +#include "base/command_line.h" #include "base/strings/string_number_conversions.h" #include "base/threading/thread_restrictions.h" #include "base/trace_event/trace_event.h" @@ -96,9 +98,18 @@ void AtomRenderFrameObserver::DidCreateScriptContext( if (ShouldNotifyClient(world_id)) renderer_client_->DidCreateScriptContext(context, render_frame_); - if (renderer_client_->isolated_world() && IsMainWorld(world_id) && - // Only the top window's main frame has isolated world. - render_frame_->IsMainFrame() && !render_frame_->GetWebFrame()->Opener()) { + bool use_context_isolation = renderer_client_->isolated_world(); + bool is_main_world = IsMainWorld(world_id); + bool is_main_frame = render_frame_->IsMainFrame(); + bool is_not_opened = !render_frame_->GetWebFrame()->Opener(); + bool allow_node_in_sub_frames = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + bool should_create_isolated_context = + use_context_isolation && is_main_world && + (is_main_frame || allow_node_in_sub_frames) && is_not_opened; + + if (should_create_isolated_context) { CreateIsolatedWorldContext(); renderer_client_->SetupMainWorldOverrides(context, render_frame_); } @@ -155,7 +166,11 @@ bool AtomRenderFrameObserver::IsIsolatedWorld(int world_id) { } bool AtomRenderFrameObserver::ShouldNotifyClient(int world_id) { - if (renderer_client_->isolated_world() && render_frame_->IsMainFrame()) + bool allow_node_in_sub_frames = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + if (renderer_client_->isolated_world() && + (render_frame_->IsMainFrame() || allow_node_in_sub_frames)) return IsIsolatedWorld(world_id); else return IsMainWorld(world_id); diff --git a/spec/api-subframe-spec.js b/spec/api-subframe-spec.js index dc31a370b3..df56873490 100644 --- a/spec/api-subframe-spec.js +++ b/spec/api-subframe-spec.js @@ -8,8 +8,8 @@ const { closeWindow } = require('./window-helpers') const { BrowserWindow } = remote describe('renderer nodeIntegrationInSubFrames', () => { - const generateTests = (sandboxEnabled) => { - describe(`with sandbox ${sandboxEnabled ? 'enabled' : 'disabled'}`, () => { + const generateTests = (description, webPreferences) => { + describe(description, () => { let w beforeEach(async () => { @@ -19,15 +19,17 @@ describe('renderer nodeIntegrationInSubFrames', () => { width: 400, height: 400, webPreferences: { - sandbox: sandboxEnabled, preload: path.resolve(__dirname, 'fixtures/sub-frames/preload.js'), - nodeIntegrationInSubFrames: true + nodeIntegrationInSubFrames: true, + ...webPreferences } }) }) afterEach(() => { - return closeWindow(w).then(() => { w = null }) + return closeWindow(w).then(() => { + w = null + }) }) it('should load preload scripts in top level iframes', async () => { @@ -74,15 +76,40 @@ describe('renderer nodeIntegrationInSubFrames', () => { it('should correctly reply to the nested sub-frames with using event.reply', async () => { const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 3) w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-with-frame-container.html')) - const [,, event3] = await detailsPromise + const [, , event3] = await detailsPromise const pongPromise = emittedOnce(remote.ipcMain, 'preload-pong') event3[0].reply('preload-ping') const details = await pongPromise expect(details[1]).to.equal(event3[0].frameId) }) + + it('should not expose globals in main world', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html')) + const details = await detailsPromise + const senders = details.map(event => event[0].sender) + await new Promise((resolve) => { + let resultCount = 0 + senders.forEach(sender => { + sender.webContents.executeJavaScript('window.isolatedGlobal', result => { + if (webPreferences.contextIsolation) { + expect(result).to.be.null() + } else { + expect(result).to.equal(true) + } + resultCount++ + if (resultCount === senders.length) { + resolve() + } + }) + }) + }) + }) }) } - generateTests(false) - generateTests(true) + generateTests('without sandbox', {}) + generateTests('with sandbox', { sandbox: true }) + generateTests('with contextIsolation', { contextIsolation: true }) + generateTests('with contextIsolation + sandbox', { contextIsolation: true, sandbox: true }) }) diff --git a/spec/fixtures/sub-frames/preload.js b/spec/fixtures/sub-frames/preload.js index 3b6c461759..11c77a5281 100644 --- a/spec/fixtures/sub-frames/preload.js +++ b/spec/fixtures/sub-frames/preload.js @@ -1,5 +1,7 @@ const { ipcRenderer, webFrame } = require('electron') +window.isolatedGlobal = true + ipcRenderer.send('preload-ran', window.location.href, webFrame.routingId) ipcRenderer.on('preload-ping', () => {