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

atom/browser/api/atom_api_web_contents.cc

@@ -1891,6 +1891,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) {
   auto* web_preferences = WebContentsPreferences::From(web_contents());
   if (!web_preferences)
@@ -2053,6 +2063,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

@@ -232,6 +232,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);
   v8::Local<v8::Value> GetLastWebPreferences(v8::Isolate* isolate);

atom/browser/web_contents_preferences.cc

@@ -109,6 +109,30 @@ void WebContentsPreferences::Merge(const base::DictionaryValue& extend) {
   dict_.MergeDictionary(&extend);
 }
 
+bool WebContentsPreferences::GetPreloadPath(
+    base::FilePath::StringType* path) const {
+  DCHECK(path);
+  base::FilePath::StringType preload;
+  if (dict_.GetString(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 (dict_.GetString(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) {
@@ -171,19 +195,8 @@ void WebContentsPreferences::AppendCommandLineSwitches(
 
   // The preload script.
   base::FilePath::StringType preload;
-  if (dict_.GetString(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 (dict_.GetString(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
   base::Value* customArgs;

atom/browser/web_contents_preferences.h

@@ -48,6 +48,9 @@ class WebContentsPreferences
   // Modify the WebPreferences according to preferences.
   void OverrideWebkitPrefs(content::WebPreferences* prefs);
 
+  // Returns the preload script path.
+  bool GetPreloadPath(base::FilePath::StringType* path) const;
+
   // Returns the web preferences.
   base::DictionaryValue* dict() { return &dict_; }
   const base::DictionaryValue* dict() const { return &dict_; }

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -169,16 +169,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(
@@ -191,10 +187,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

@@ -450,7 +450,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) {

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, webContentsId, platform, env
-} = electron.ipcRenderer.sendSync('ELECTRON_BROWSER_SANDBOX_LOAD', preloadPath)
+} = electron.ipcRenderer.sendSync('ELECTRON_BROWSER_SANDBOX_LOAD')
 
 Object.defineProperty(process, 'webContentsId', {
   configurable: false,

spec/api-browser-window-spec.js

@@ -1363,6 +1363,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
@@ -1541,6 +1543,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) => {