Don't pass preloadPath via ELECTRON_BROWSER_SANDBOX_LOAD for security reasons (#13031)

atom/browser/api/atom_api_web_contents.cc

@@ -1884,6 +1884,16 @@ void WebContents::OnGetZoomLevel(content::RenderFrameHost* rfh,
   rfh->Send(reply_msg);
 }
 
+v8::Local<v8::Value> WebContents::GetPreloadPath(v8::Isolate* isolate) const {
+  if (auto* web_preferences = WebContentsPreferences::From(web_contents())) {
+    base::FilePath::StringType preload;
+    if (web_preferences->GetPreloadPath(&preload)) {
+      return mate::ConvertToV8(isolate, preload);
+    }
+  }
+  return v8::Null(isolate);
+}
+
 v8::Local<v8::Value> WebContents::GetWebPreferences(
     v8::Isolate* isolate) const {
   auto* web_preferences = WebContentsPreferences::From(web_contents());
@@ -2047,6 +2057,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setZoomFactor", &WebContents::SetZoomFactor)
       .SetMethod("_getZoomFactor", &WebContents::GetZoomFactor)
       .SetMethod("getType", &WebContents::GetType)
+      .SetMethod("_getPreloadPath", &WebContents::GetPreloadPath)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)
       .SetMethod("getLastWebPreferences", &WebContents::GetLastWebPreferences)
       .SetMethod("getOwnerBrowserWindow", &WebContents::GetOwnerBrowserWindow)

atom/browser/api/atom_api_web_contents.h

@@ -230,6 +230,9 @@ class WebContents : public mate::TrackableObject<WebContents>,
                       const std::vector<std::string>& features,
                       const scoped_refptr<network::ResourceRequestBody>& body);
 
+  // Returns the preload script path of current WebContents.
+  v8::Local<v8::Value> GetPreloadPath(v8::Isolate* isolate) const;
+
   // Returns the web preferences of current WebContents.
   v8::Local<v8::Value> GetWebPreferences(v8::Isolate* isolate) const;
   v8::Local<v8::Value> GetLastWebPreferences(v8::Isolate* isolate) const;

atom/browser/web_contents_preferences.cc

@@ -169,6 +169,30 @@ bool WebContentsPreferences::GetPreference(const base::StringPiece& name,
   return GetAsString(&preference_, name, value);
 }
 
+bool WebContentsPreferences::GetPreloadPath(
+    base::FilePath::StringType* path) const {
+  DCHECK(path);
+  base::FilePath::StringType preload;
+  if (GetAsString(&preference_, options::kPreloadScript, &preload)) {
+    if (base::FilePath(preload).IsAbsolute()) {
+      *path = std::move(preload);
+      return true;
+    } else {
+      LOG(ERROR) << "preload script must have absolute path.";
+    }
+  } else if (GetAsString(&preference_, options::kPreloadURL, &preload)) {
+    // Translate to file path if there is "preload-url" option.
+    base::FilePath preload_path;
+    if (net::FileURLToFilePath(GURL(preload), &preload_path)) {
+      *path = std::move(preload_path.value());
+      return true;
+    } else {
+      LOG(ERROR) << "preload url must be file:// protocol.";
+    }
+  }
+  return false;
+}
+
 // static
 content::WebContents* WebContentsPreferences::GetWebContentsFromProcessID(
     int process_id) {
@@ -228,19 +252,8 @@ void WebContentsPreferences::AppendCommandLineSwitches(
 
   // The preload script.
   base::FilePath::StringType preload;
-  if (GetAsString(&preference_, options::kPreloadScript, &preload)) {
+  if (GetPreloadPath(&preload))
-    if (base::FilePath(preload).IsAbsolute())
+    command_line->AppendSwitchNative(switches::kPreloadScript, preload);
-      command_line->AppendSwitchNative(switches::kPreloadScript, preload);
-    else
-      LOG(ERROR) << "preload script must have absolute path.";
-  } else if (GetAsString(&preference_, options::kPreloadURL, &preload)) {
-    // Translate to file path if there is "preload-url" option.
-    base::FilePath preload_path;
-    if (net::FileURLToFilePath(GURL(preload), &preload_path))
-      command_line->AppendSwitchPath(switches::kPreloadScript, preload_path);
-    else
-      LOG(ERROR) << "preload url must be file:// protocol.";
-  }
 
   // Custom args for renderer process
   auto* customArgs =

atom/browser/web_contents_preferences.h

@@ -55,6 +55,9 @@ class WebContentsPreferences
   // Return true if the particular preference value exists.
   bool GetPreference(const base::StringPiece& name, std::string* value) const;
 
+  // Returns the preload script path.
+  bool GetPreloadPath(base::FilePath::StringType* path) const;
+
   // Returns the web preferences.
   base::Value* preference() { return &preference_; }
   base::Value* last_preference() { return &last_preference_; }

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -177,16 +177,12 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
   if (!render_frame->IsMainFrame() && !IsDevTools(render_frame))
     return;
 
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  base::FilePath preload_script_path =
-      command_line->GetSwitchValuePath(switches::kPreloadScript);
-
   auto* isolate = context->GetIsolate();
   v8::HandleScope handle_scope(isolate);
   v8::Context::Scope context_scope(context);
   // Wrap the bundle into a function that receives the binding object and the
   // preload script path as arguments.
-  std::string left = "(function(binding, preloadPath, require) {\n";
+  std::string left = "(function(binding, require) {\n";
   std::string right = "\n})";
   // Compile the wrapper and run it to get the function object
   auto script = v8::Script::Compile(v8::String::Concat(
@@ -199,10 +195,10 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
   auto binding = v8::Object::New(isolate);
   InitializeBindings(binding, context);
   AddRenderBindings(isolate, binding);
-  v8::Local<v8::Value> args[] = {
+  v8::Local<v8::Value> args[] = {binding};
-      binding, mate::ConvertToV8(isolate, preload_script_path.value())};
   // Execute the function with proper arguments
-  ignore_result(func->Call(context, v8::Null(isolate), 2, args));
+  ignore_result(
+      func->Call(context, v8::Null(isolate), node::arraysize(args), args));
 }
 
 void AtomSandboxedRendererClient::WillReleaseScriptContext(

lib/browser/rpc-server.js

@@ -440,7 +440,8 @@ ipcMain.on('ELECTRON_BROWSER_WINDOW_CLOSE', function (event) {
   event.returnValue = null
 })
 
-ipcMain.on('ELECTRON_BROWSER_SANDBOX_LOAD', function (event, preloadPath) {
+ipcMain.on('ELECTRON_BROWSER_SANDBOX_LOAD', function (event) {
+  const preloadPath = event.sender._getPreloadPath()
   let preloadSrc = null
   let preloadError = null
   if (preloadPath) {
@@ -451,8 +452,8 @@ ipcMain.on('ELECTRON_BROWSER_SANDBOX_LOAD', function (event, preloadPath) {
     }
   }
   event.returnValue = {
-    preloadSrc: preloadSrc,
+    preloadSrc,
-    preloadError: preloadError,
+    preloadError,
     platform: process.platform,
     env: process.env
   }

lib/sandboxed_renderer/init.js

@@ -1,5 +1,5 @@
 /* eslint no-eval: "off" */
-/* global binding, preloadPath, Buffer */
+/* global binding, Buffer */
 const events = require('events')
 const electron = require('electron')
 
@@ -37,7 +37,7 @@ const loadedModules = new Map([
 
 const {
   preloadSrc, preloadError, platform, env
-} = electron.ipcRenderer.sendSync('ELECTRON_BROWSER_SANDBOX_LOAD', preloadPath)
+} = electron.ipcRenderer.sendSync('ELECTRON_BROWSER_SANDBOX_LOAD')
 
 require('../renderer/web-frame-init')()
 

spec/api-browser-window-spec.js

@@ -1409,6 +1409,8 @@ describe('BrowserWindow module', () => {
             preload: preload
           }
         })
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload)
+
         let htmlPath = path.join(fixtures, 'api', 'sandbox.html?verify-ipc-sender')
         const pageUrl = 'file://' + htmlPath
         let childWc
@@ -1587,6 +1589,8 @@ describe('BrowserWindow module', () => {
             sandbox: true
           }
         })
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload)
+
         w.loadURL('file://' + path.join(fixtures, 'api', 'sandbox.html?reload-remote-child'))
 
         ipcMain.on('get-remote-module-path', (event) => {