Merge pull request #10214 from psh0628/contextisolation-sandbox-fix

fix contextIsolation issue while webPreference sandbox is on

atom/renderer/atom_renderer_client.cc

@@ -41,8 +41,6 @@ AtomRendererClient::AtomRendererClient()
     : node_integration_initialized_(false),
       node_bindings_(NodeBindings::Create(NodeBindings::RENDERER)),
       atom_bindings_(new AtomBindings(uv_default_loop())) {
-  isolated_world_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kContextIsolation);
 }
 
 AtomRendererClient::~AtomRendererClient() {
@@ -171,14 +169,6 @@ void AtomRendererClient::WillDestroyWorkerContextOnWorkerThread(
   }
 }
 
-v8::Local<v8::Context> AtomRendererClient::GetContext(
-    blink::WebFrame* frame, v8::Isolate* isolate) {
-  if (isolated_world())
-    return frame->WorldScriptContext(isolate, World::ISOLATED_WORLD);
-  else
-    return frame->MainWorldScriptContext();
-}
-
 void AtomRendererClient::SetupMainWorldOverrides(
     v8::Handle<v8::Context> context) {
   // Setup window overrides in the main world context

atom/renderer/atom_renderer_client.h

@@ -20,10 +20,6 @@ class AtomRendererClient : public RendererClientBase {
   AtomRendererClient();
   virtual ~AtomRendererClient();
 
-  // Get the context that the Electron API is running in.
-  v8::Local<v8::Context> GetContext(
-      blink::WebFrame* frame, v8::Isolate* isolate);
-
   // atom::RendererClientBase:
   void DidCreateScriptContext(
       v8::Handle<v8::Context> context,
@@ -32,7 +28,6 @@ class AtomRendererClient : public RendererClientBase {
       v8::Handle<v8::Context> context,
       content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override;
-  bool isolated_world() override { return isolated_world_; }
 
  private:
   enum NodeIntegration {
@@ -64,7 +59,6 @@ class AtomRendererClient : public RendererClientBase {
 
   std::unique_ptr<NodeBindings> node_bindings_;
   std::unique_ptr<AtomBindings> atom_bindings_;
-  bool isolated_world_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomRendererClient);
 };

atom/renderer/atom_sandboxed_renderer_client.h

@@ -27,7 +27,6 @@ class AtomSandboxedRendererClient : public RendererClientBase {
       v8::Handle<v8::Context> context,
       content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override { }
-  bool isolated_world() override { return false; }
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;

atom/renderer/renderer_client_base.cc

@@ -69,6 +69,8 @@ RendererClientBase::RendererClientBase() {
       ParseSchemesCLISwitch(switches::kStandardSchemes);
   for (const std::string& scheme : standard_schemes_list)
     url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
+    isolated_world_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kContextIsolation);
 }
 
 RendererClientBase::~RendererClientBase() {
@@ -197,4 +199,12 @@ void RendererClientBase::AddSupportedKeySystems(
   AddChromeKeySystems(key_systems);
 }
 
+v8::Local<v8::Context> RendererClientBase::GetContext(
+    blink::WebFrame* frame, v8::Isolate* isolate) {
+  if (isolated_world())
+    return frame->WorldScriptContext(isolate, World::ISOLATED_WORLD);
+  else
+    return frame->MainWorldScriptContext();
+}
+
 }  // namespace atom

atom/renderer/renderer_client_base.h

@@ -25,7 +25,12 @@ class RendererClientBase : public content::ContentRendererClient {
       v8::Handle<v8::Context> context, content::RenderFrame* render_frame) = 0;
   virtual void DidClearWindowObject(content::RenderFrame* render_frame);
   virtual void SetupMainWorldOverrides(v8::Handle<v8::Context> context) = 0;
-  virtual bool isolated_world() = 0;
+
+  bool isolated_world() { return isolated_world_; }
+
+  // Get the context that the Electron API is running in.
+  v8::Local<v8::Context> GetContext(
+      blink::WebFrame* frame, v8::Isolate* isolate);
 
  protected:
   void AddRenderBindings(v8::Isolate* isolate,
@@ -51,6 +56,7 @@ class RendererClientBase : public content::ContentRendererClient {
 
  private:
   std::unique_ptr<PreferencesManager> preferences_manager_;
+  bool isolated_world_;
 };
 
 }  // namespace atom

docs/api/sandbox-option.md

@@ -166,7 +166,11 @@ Currently the `require` function provided in the preload scope exposes the
 following modules:
 
 - `child_process`
-- `electron` (crashReporter, remote and ipcRenderer)
+- `electron`
+  - `crashReporter`
+  - `remote`
+  - `ipcRenderer`
+  - `webFrame`
 - `fs`
 - `os`
 - `timers`

lib/sandboxed_renderer/api/exports/electron.js

@@ -11,6 +11,12 @@ Object.defineProperties(exports, {
       return require('../../../renderer/api/remote')
     }
   },
+  webFrame: {
+    enumerable: true,
+    get: function () {
+      return require('../../../renderer/api/web-frame')
+    }
+  },
   crashReporter: {
     enumerable: true,
     get: function () {

spec/api-browser-window-spec.js

@@ -17,6 +17,7 @@ const nativeModulesEnabled = remote.getGlobal('nativeModulesEnabled')
 describe('BrowserWindow module', function () {
   var fixtures = path.resolve(__dirname, 'fixtures')
   var w = null
+  var ws = null
   var server, postData
 
   before(function (done) {
@@ -2643,7 +2644,7 @@ describe('BrowserWindow module', function () {
     })
   })
 
-  describe('contextIsolation option', () => {
+  describe('contextIsolation option with and without sandbox option', () => {
     const expectedContextData = {
       preloadContext: {
         preloadProperty: 'number',
@@ -2674,6 +2675,19 @@ describe('BrowserWindow module', function () {
           preload: path.join(fixtures, 'api', 'isolated-preload.js')
         }
       })
+      if (ws != null) ws.destroy()
+      ws = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          sandbox: true,
+          contextIsolation: true,
+          preload: path.join(fixtures, 'api', 'isolated-preload.js')
+        }
+      })
+    })
+
+    afterEach(() => {
+      if (ws != null) ws.destroy()
     })
 
     it('separates the page context from the Electron/preload context', (done) => {
@@ -2702,6 +2716,25 @@ describe('BrowserWindow module', function () {
       })
       w.loadURL('file://' + fixtures + '/pages/window-open.html')
     })
+
+    it('separates the page context from the Electron/preload context with sandbox on', (done) => {
+      ipcMain.once('isolated-sandbox-world', (event, data) => {
+        assert.deepEqual(data, expectedContextData)
+        done()
+      })
+      w.loadURL('file://' + fixtures + '/api/isolated.html')
+    })
+
+    it('recreates the contexts on reload with sandbox on', (done) => {
+      w.webContents.once('did-finish-load', () => {
+        ipcMain.once('isolated-sandbox-world', (event, data) => {
+          assert.deepEqual(data, expectedContextData)
+          done()
+        })
+        w.webContents.reload()
+      })
+      w.loadURL('file://' + fixtures + '/api/isolated.html')
+    })
   })
 
   describe('offscreen rendering', function () {

spec/fixtures/api/isolated-preload.js

@@ -1,7 +1,6 @@
 // Ensure fetch works from isolated world origin
 fetch('http://localhost:1234')
 fetch('https://localhost:1234')
-fetch(`file://${__filename}`)
 
 const {ipcRenderer, webFrame} = require('electron')
 
@@ -21,4 +20,15 @@ window.addEventListener('message', (event) => {
     },
     pageContext: event.data
   })
+  ipcRenderer.send('isolated-sandbox-world', {
+    preloadContext: {
+      preloadProperty: typeof window.foo,
+      pageProperty: typeof window.hello,
+      typeofRequire: typeof require,
+      typeofProcess: typeof process,
+      typeofArrayPush: typeof Array.prototype.push,
+      typeofFunctionApply: typeof Function.prototype.apply
+    },
+    pageContext: event.data
+  })
 })