Home Explore 引导页 在线工具 Blog
Register Sign In
Pchen0
/
electron
1
0
Fork 0
Files Issues 0 Pull Requests 0 Wiki
Browse Source

fix: illegal access errors with nodeIntegrationInSubFrames (#29169)

* fix: illegal access errors with nodeIntegrationInSubFrames

* test: fixup on windows

* Address feedback from review

* Update shell/renderer/electron_render_frame_observer.cc

Co-authored-by: Robo <[email protected]>

Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: Robo <[email protected]>
trop[bot] 3 years ago
parent
4b439def13
commit
8c23e08e67
6 changed files with 104 additions and 4 deletions
Split View Show Diff Stats
  1. 15 2
      shell/renderer/electron_render_frame_observer.cc
  2. 3 2
      shell/renderer/electron_renderer_client.cc
  3. 2 0
      shell/renderer/electron_sandboxed_renderer_client.cc
  4. 29 0
      spec-main/fixtures/crash-cases/js-execute-iframe/index.html
  5. 51 0
      spec-main/fixtures/crash-cases/js-execute-iframe/index.js
  6. 4 0
      spec-main/fixtures/crash-cases/js-execute-iframe/page2.html

+ 15 - 2
shell/renderer/electron_render_frame_observer.cc
View File 

@@ -78,6 +78,7 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
 
   bool is_not_opened = !render_frame_->GetWebFrame()->Opener() ||
 
                        prefs.node_leakage_in_renderers;
 
   bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames;
 
+
 
   bool should_create_isolated_context =
 
       use_context_isolation && is_main_world &&
 
       (is_main_frame || allow_node_in_sub_frames) &&
 
@@ -163,12 +164,24 @@ bool ElectronRenderFrameObserver::IsIsolatedWorld(int world_id) {
 
 
 bool ElectronRenderFrameObserver::ShouldNotifyClient(int world_id) {
 
   auto prefs = render_frame_->GetBlinkPreferences();
 
+
 
+  // This is necessary because if an iframe is created and a source is not
 
+  // set, the iframe loads about:blank and creates a script context for the
 
+  // same. We don't want to create a Node.js environment here because if the src
 
+  // is later set, the JS necessary to do that triggers illegal access errors
 
+  // when the initial about:blank Node.js environment is cleaned up. See:
 
+  // https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/render_frame_impl.h;l=870-892;drc=4b6001440a18740b76a1c63fa2a002cc941db394
 
+  GURL url = render_frame_->GetWebFrame()->GetDocument().Url();
 
   bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames;
 
+  if (allow_node_in_sub_frames && url.IsAboutBlank() &&
 
+      !render_frame_->IsMainFrame())
 
+    return false;
 
+
 
   if (prefs.context_isolation &&
 
       (render_frame_->IsMainFrame() || allow_node_in_sub_frames))
 
     return IsIsolatedWorld(world_id);
 
-  else
 
-    return IsMainWorld(world_id);
 
+
 
+  return IsMainWorld(world_id);
 
 }
 
 
 }  // namespace electron

+ 3 - 2
shell/renderer/electron_renderer_client.cc
View File 

@@ -80,8 +80,8 @@ void ElectronRendererClient::DidCreateScriptContext(
 
   // TODO(zcbenz): Do not create Node environment if node integration is not
 
   // enabled.
 
 
-  // Only load node if we are a main frame or a devtools extension
 
-  // unless node support has been explicitly enabled for sub frames
 
+  // Only load Node.js if we are a main frame or a devtools extension
 
+  // unless Node.js support has been explicitly enabled for subframes.
 
   auto prefs = render_frame->GetBlinkPreferences();
 
   bool reuse_renderer_processes_enabled =
 
       prefs.disable_electron_site_instance_overrides;
 
@@ -97,6 +97,7 @@ void ElectronRendererClient::DidCreateScriptContext(
 
                        (is_not_opened || reuse_renderer_processes_enabled);
 
   bool is_devtools = IsDevToolsExtension(render_frame);
 
   bool allow_node_in_subframes = prefs.node_integration_in_sub_frames;
 
+
 
   bool should_load_node =
 
       (is_main_frame || is_devtools || allow_node_in_subframes) &&
 
       !IsWebViewFrame(renderer_context, render_frame);

+ 2 - 0
shell/renderer/electron_sandboxed_renderer_client.cc
View File 

@@ -198,8 +198,10 @@ void ElectronSandboxedRendererClient::DidCreateScriptContext(
 
   bool is_main_frame = render_frame->IsMainFrame();
 
   bool is_devtools =
 
       IsDevTools(render_frame) || IsDevToolsExtension(render_frame);
 
+
 
   bool allow_node_in_sub_frames =
 
       render_frame->GetBlinkPreferences().node_integration_in_sub_frames;
 
+
 
   bool should_load_preload =
 
       (is_main_frame || is_devtools || allow_node_in_sub_frames) &&
 
       !IsWebViewFrame(context, render_frame);

+ 29 - 0
spec-main/fixtures/crash-cases/js-execute-iframe/index.html
View File 

@@ -0,0 +1,29 @@
 
+<html>
 
+  <body>
 
+    <iframe id="mainframe"></iframe>
 
+    <script>
 
+      const net = require('net');
 
+      const path = require('path');
 
+
 
+      document.getElementById("mainframe").src="./page2.html";
 
+
 
+      const p = process.platform === 'win32'
 
+              ? path.join('\\\\?\\pipe', process.cwd(), 'myctl')
 
+              : '/tmp/echo.sock';
 
+
 
+      const client = net.createConnection({ path: p }, () => {
 
+        console.log('connected to server');
 
+        client.write('world!\r\n');
 
+      });
 
+
 
+      client.on('data', (data) => {
 
+        console.log(data.toString());
 
+        client.end();
 
+      });
 
+
 
+      client.on('end', () => {
 
+        console.log('disconnected from server');
 
+      });
 
+    </script>
 
+  </body>
 
+</html>

+ 51 - 0
spec-main/fixtures/crash-cases/js-execute-iframe/index.js
View File 

@@ -0,0 +1,51 @@
 
+const { app, BrowserWindow } = require('electron');
 
+const net = require('net');
 
+const path = require('path');
 
+
 
+function createWindow () {
 
+  const mainWindow = new BrowserWindow({
 
+    webPreferences: {
 
+      nodeIntegration: true,
 
+      contextIsolation: false,
 
+      nodeIntegrationInSubFrames: true
 
+    }
 
+  });
 
+
 
+  mainWindow.loadFile('index.html');
 
+}
 
+
 
+app.whenReady().then(() => {
 
+  createWindow();
 
+
 
+  app.on('activate', () => {
 
+    if (BrowserWindow.getAllWindows().length === 0) createWindow();
 
+  });
 
+});
 
+
 
+app.on('window-all-closed', () => {
 
+  if (process.platform !== 'darwin') app.quit();
 
+});
 
+
 
+const server = net.createServer((c) => {
 
+  console.log('client connected');
 
+
 
+  c.on('end', () => {
 
+    console.log('client disconnected');
 
+    app.quit();
 
+  });
 
+
 
+  c.write('hello\r\n');
 
+  c.pipe(c);
 
+});
 
+
 
+server.on('error', (err) => {
 
+  throw err;
 
+});
 
+
 
+const p = process.platform === 'win32'
 
+  ? path.join('\\\\?\\pipe', process.cwd(), 'myctl')
 
+  : '/tmp/echo.sock';
 
+
 
+server.listen(p, () => {
 
+  console.log('server bound');
 
+});

+ 4 - 0
spec-main/fixtures/crash-cases/js-execute-iframe/page2.html
View File 

@@ -0,0 +1,4 @@
 
+<!DOCTYPE html>
 
+<html>
 
+	<body>HELLO</body>
 
+</html>