fix: ignore non-absolute session preload script paths when sandboxed (#19066)

lib/browser/rpc-server.js

@@ -519,12 +519,10 @@ if (features.isDesktopCapturerEnabled()) {
 const getPreloadScript = async function (preloadPath) {
   let preloadSrc = null
   let preloadError = null
-  if (preloadPath) {
-    try {
-      preloadSrc = (await fs.promises.readFile(preloadPath)).toString()
-    } catch (err) {
-      preloadError = errorUtils.serialize(err)
-    }
+  try {
+    preloadSrc = (await fs.promises.readFile(preloadPath)).toString()
+  } catch (err) {
+    preloadError = errorUtils.serialize(err)
   }
   return { preloadPath, preloadSrc, preloadError }
 }
@@ -532,10 +530,7 @@ const getPreloadScript = async function (preloadPath) {
 ipcMainUtils.handle('ELECTRON_GET_CONTENT_SCRIPTS', () => getContentScripts())
 
 ipcMainUtils.handle('ELECTRON_BROWSER_SANDBOX_LOAD', async function (event) {
-  const preloadPaths = [
-    ...(event.sender.session ? event.sender.session.getPreloads() : []),
-    event.sender._getPreloadPath()
-  ]
+  const preloadPaths = event.sender._getPreloadPaths()
 
   return {
     contentScripts: getContentScripts(),

shell/browser/api/atom_api_web_contents.cc

@@ -60,6 +60,7 @@
 #include "shell/browser/lib/bluetooth_chooser.h"
 #include "shell/browser/native_window.h"
 #include "shell/browser/net/atom_network_delegate.h"
+#include "shell/browser/session_preferences.h"
 #include "shell/browser/ui/drag_util.h"
 #include "shell/browser/ui/inspectable_web_contents.h"
 #include "shell/browser/ui/inspectable_web_contents_view.h"
@@ -2206,14 +2207,17 @@ void WebContents::HideAutofillPopup() {
   CommonWebContentsDelegate::HideAutofillPopup();
 }
 
-v8::Local<v8::Value> WebContents::GetPreloadPath(v8::Isolate* isolate) const {
+std::vector<base::FilePath::StringType> WebContents::GetPreloadPaths() const {
+  auto result = SessionPreferences::GetValidPreloads(GetBrowserContext());
+
   if (auto* web_preferences = WebContentsPreferences::From(web_contents())) {
     base::FilePath::StringType preload;
     if (web_preferences->GetPreloadPath(&preload)) {
-      return mate::ConvertToV8(isolate, preload);
+      result.emplace_back(preload);
     }
   }
-  return v8::Null(isolate);
+
+  return result;
 }
 
 v8::Local<v8::Value> WebContents::GetWebPreferences(
@@ -2437,7 +2441,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setZoomFactor", &WebContents::SetZoomFactor)
       .SetMethod("getZoomFactor", &WebContents::GetZoomFactor)
       .SetMethod("getType", &WebContents::GetType)
-      .SetMethod("_getPreloadPath", &WebContents::GetPreloadPath)
+      .SetMethod("_getPreloadPaths", &WebContents::GetPreloadPaths)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)
       .SetMethod("getLastWebPreferences", &WebContents::GetLastWebPreferences)
       .SetMethod("_isRemoteModuleEnabled", &WebContents::IsRemoteModuleEnabled)

shell/browser/api/atom_api_web_contents.h

@@ -285,7 +285,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
                       const scoped_refptr<network::ResourceRequestBody>& body);
 
   // Returns the preload script path of current WebContents.
-  v8::Local<v8::Value> GetPreloadPath(v8::Isolate* isolate) const;
+  std::vector<base::FilePath::StringType> GetPreloadPaths() const;
 
   // Returns the web preferences of current WebContents.
   v8::Local<v8::Value> GetWebPreferences(v8::Isolate* isolate) const;

shell/browser/atom_browser_client.cc

@@ -151,6 +151,12 @@ void SetApplicationLocaleOnIOThread(const std::string& locale) {
   g_io_thread_application_locale.Get() = locale;
 }
 
+#if defined(OS_WIN)
+const base::FilePath::StringPieceType kPathDelimiter = FILE_PATH_LITERAL(";");
+#else
+const base::FilePath::StringPieceType kPathDelimiter = FILE_PATH_LITERAL(":");
+#endif
+
 }  // namespace
 
 // static
@@ -539,8 +545,12 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
     if (web_preferences)
       web_preferences->AppendCommandLineSwitches(
           command_line, IsRendererSubFrame(process_id));
-    SessionPreferences::AppendExtraCommandLineSwitches(
-        web_contents->GetBrowserContext(), command_line);
+    auto preloads =
+        SessionPreferences::GetValidPreloads(web_contents->GetBrowserContext());
+    if (!preloads.empty())
+      command_line->AppendSwitchNative(
+          switches::kPreloadScripts,
+          base::JoinString(preloads, kPathDelimiter));
     if (CanUseCustomSiteInstance()) {
       command_line->AppendSwitch(
           switches::kDisableElectronSiteInstanceOverrides);

shell/browser/session_preferences.cc

@@ -4,22 +4,8 @@
 
 #include "shell/browser/session_preferences.h"
 
-#include "base/command_line.h"
-#include "base/memory/ptr_util.h"
-#include "shell/common/options_switches.h"
-
 namespace electron {
 
-namespace {
-
-#if defined(OS_WIN)
-const base::FilePath::CharType kPathDelimiter = FILE_PATH_LITERAL(';');
-#else
-const base::FilePath::CharType kPathDelimiter = FILE_PATH_LITERAL(':');
-#endif
-
-}  // namespace
-
 // static
 int SessionPreferences::kLocatorKey = 0;
 
@@ -36,26 +22,21 @@ SessionPreferences* SessionPreferences::FromBrowserContext(
 }
 
 // static
-void SessionPreferences::AppendExtraCommandLineSwitches(
-    content::BrowserContext* context,
-    base::CommandLine* command_line) {
-  SessionPreferences* self = FromBrowserContext(context);
-  if (!self)
-    return;
-
-  base::FilePath::StringType preloads;
-  for (const auto& preload : self->preloads()) {
-    if (!base::FilePath(preload).IsAbsolute()) {
-      LOG(ERROR) << "preload script must have absolute path: " << preload;
-      continue;
+std::vector<base::FilePath::StringType> SessionPreferences::GetValidPreloads(
+    content::BrowserContext* context) {
+  std::vector<base::FilePath::StringType> result;
+
+  if (auto* self = FromBrowserContext(context)) {
+    for (const auto& preload : self->preloads()) {
+      if (base::FilePath(preload).IsAbsolute()) {
+        result.emplace_back(preload);
+      } else {
+        LOG(ERROR) << "preload script must have absolute path: " << preload;
+      }
     }
-    if (preloads.empty())
-      preloads = preload;
-    else
-      preloads += kPathDelimiter + preload;
   }
-  if (!preloads.empty())
-    command_line->AppendSwitchNative(switches::kPreloadScripts, preloads);
+
+  return result;
 }
 
 }  // namespace electron

shell/browser/session_preferences.h

@@ -11,18 +11,14 @@
 #include "base/supports_user_data.h"
 #include "content/public/browser/browser_context.h"
 
-namespace base {
-class CommandLine;
-}
-
 namespace electron {
 
 class SessionPreferences : public base::SupportsUserData::Data {
  public:
   static SessionPreferences* FromBrowserContext(
       content::BrowserContext* context);
-  static void AppendExtraCommandLineSwitches(content::BrowserContext* context,
-                                             base::CommandLine* command_line);
+  static std::vector<base::FilePath::StringType> GetValidPreloads(
+      content::BrowserContext* context);
 
   explicit SessionPreferences(content::BrowserContext* context);
   ~SessionPreferences() override;

spec/api-browser-window-spec.js

@@ -545,7 +545,8 @@ describe('BrowserWindow module', () => {
     describe('session preload scripts', function () {
       const preloads = [
         path.join(fixtures, 'module', 'set-global-preload-1.js'),
-        path.join(fixtures, 'module', 'set-global-preload-2.js')
+        path.join(fixtures, 'module', 'set-global-preload-2.js'),
+        path.relative(process.cwd(), path.join(fixtures, 'module', 'set-global-preload-3.js'))
       ]
       const defaultSession = session.defaultSession
 
@@ -564,9 +565,10 @@ describe('BrowserWindow module', () => {
       const generateSpecs = (description, sandbox) => {
         describe(description, () => {
           it('loads the script before other scripts in window including normal preloads', function (done) {
-            ipcMain.once('vars', function (event, preload1, preload2) {
+            ipcMain.once('vars', function (event, preload1, preload2, preload3) {
               expect(preload1).to.equal('preload-1')
               expect(preload2).to.equal('preload-1-2')
+              expect(preload3).to.be.null()
               done()
             })
             w.destroy()

spec/fixtures/api/preloads.html

@@ -1,7 +0,0 @@
-<html>
-<body>
-<script type="text/javascript" charset="utf-8">
-require('electron').ipcRenderer.send('vars', preload1, preload2, preload3);
-</script>
-</body>
-</html>

spec/fixtures/module/get-global-preload.js

@@ -1 +1 @@
-require('electron').ipcRenderer.send('vars', window.preload1, window.preload2)
+require('electron').ipcRenderer.send('vars', window.preload1, window.preload2, window.preload3)

spec/fixtures/module/set-global-preload-3.js

@@ -0,0 +1 @@
+window.preload3 = window.preload2 + '-3'