fix: webFrame spell checker APIs crashing in sandboxed renderers (#29053)

shell/renderer/api/electron_api_web_frame.cc

@@ -33,7 +33,7 @@
 #include "shell/renderer/api/context_bridge/object_cache.h"
 #include "shell/renderer/api/electron_api_context_bridge.h"
 #include "shell/renderer/api/electron_api_spell_check_client.h"
-#include "shell/renderer/electron_renderer_client.h"
+#include "shell/renderer/renderer_client_base.h"
 #include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/common/page/page_zoom.h"
 #include "third_party/blink/public/common/web_cache/web_cache_resource_type_stats.h"
@@ -121,7 +121,7 @@ bool SpellCheckWord(content::RenderFrame* render_frame,
   size_t start;
   size_t length;
 
-  ElectronRendererClient* client = ElectronRendererClient::Get();
+  RendererClientBase* client = RendererClientBase::Get();
 
   std::u16string w = base::UTF8ToUTF16(word);
   int id = render_frame->GetRoutingID();

shell/renderer/electron_renderer_client.cc

@@ -35,27 +35,15 @@ bool IsDevToolsExtension(content::RenderFrame* render_frame) {
 
 }  // namespace
 
-// static
-ElectronRendererClient* ElectronRendererClient::self_ = nullptr;
-
 ElectronRendererClient::ElectronRendererClient()
     : node_bindings_(
           NodeBindings::Create(NodeBindings::BrowserEnvironment::kRenderer)),
-      electron_bindings_(new ElectronBindings(node_bindings_->uv_loop())) {
+      electron_bindings_(new ElectronBindings(node_bindings_->uv_loop())) {}
-  DCHECK(!self_) << "Cannot have two ElectronRendererClient";
-  self_ = this;
-}
 
 ElectronRendererClient::~ElectronRendererClient() {
   asar::ClearArchives();
 }
 
-// static
-ElectronRendererClient* ElectronRendererClient::Get() {
-  DCHECK(self_);
-  return self_;
-}
-
 void ElectronRendererClient::RenderFrameCreated(
     content::RenderFrame* render_frame) {
   new ElectronRenderFrameObserver(render_frame, this);

shell/renderer/electron_renderer_client.h

@@ -26,8 +26,6 @@ class ElectronRendererClient : public RendererClientBase {
   ElectronRendererClient();
   ~ElectronRendererClient() override;
 
-  static ElectronRendererClient* Get();
-
   // electron::RendererClientBase:
   void DidCreateScriptContext(v8::Handle<v8::Context> context,
                               content::RenderFrame* render_frame) override;
@@ -68,8 +66,6 @@ class ElectronRendererClient : public RendererClientBase {
   // assertion, so we have to keep a book of injected web frames.
   std::set<content::RenderFrame*> injected_frames_;
 
-  static ElectronRendererClient* self_;
-
   DISALLOW_COPY_AND_ASSIGN(ElectronRendererClient);
 };
 

shell/renderer/renderer_client_base.cc

@@ -92,6 +92,9 @@ std::vector<std::string> ParseSchemesCLISwitch(base::CommandLine* command_line,
                            base::SPLIT_WANT_NONEMPTY);
 }
 
+// static
+RendererClientBase* g_renderer_client_base = nullptr;
+
 }  // namespace
 
 RendererClientBase::RendererClientBase() {
@@ -123,9 +126,13 @@ RendererClientBase::RendererClientBase() {
   DCHECK(command_line->HasSwitch(::switches::kRendererClientId));
   renderer_client_id_ =
       command_line->GetSwitchValueASCII(::switches::kRendererClientId);
+
+  g_renderer_client_base = this;
 }
 
-RendererClientBase::~RendererClientBase() = default;
+RendererClientBase::~RendererClientBase() {
+  g_renderer_client_base = nullptr;
+}
 
 void RendererClientBase::DidCreateScriptContext(
     v8::Handle<v8::Context> context,
@@ -137,6 +144,12 @@ void RendererClientBase::DidCreateScriptContext(
   global.SetHidden("contextId", context_id);
 }
 
+// static
+RendererClientBase* RendererClientBase::Get() {
+  DCHECK(g_renderer_client_base);
+  return g_renderer_client_base;
+}
+
 void RendererClientBase::BindProcess(v8::Isolate* isolate,
                                      gin_helper::Dictionary* process,
                                      content::RenderFrame* render_frame) {

shell/renderer/renderer_client_base.h

@@ -60,6 +60,8 @@ class RendererClientBase : public content::ContentRendererClient
   RendererClientBase();
   ~RendererClientBase() override;
 
+  static RendererClientBase* Get();
+
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
   // service_manager::LocalInterfaceProvider implementation.
   void GetInterface(const std::string& name,

spec-main/spellchecker-spec.ts

@@ -62,13 +62,19 @@ ifdescribe(features.isBuiltinSpellCheckerEnabled())('spellchecker', function ()
   });
   after(() => server.close());
 
+  const fixtures = path.resolve(__dirname, '../spec/fixtures');
+  const preload = path.join(fixtures, 'module', 'preload-electron.js');
+
+  const generateSpecs = (description: string, sandbox: boolean) => {
+    describe(description, () => {
       beforeEach(async () => {
         w = new BrowserWindow({
           show: false,
           webPreferences: {
-        nodeIntegration: true,
             partition: `unique-spell-${Date.now()}`,
-        contextIsolation: false
+            contextIsolation: false,
+            preload,
+            sandbox
           }
         });
         w.webContents.session.setSpellCheckerDictionaryDownloadURL(`http://127.0.0.1:${(server.address() as AddressInfo).port}/`);
@@ -115,7 +121,7 @@ ifdescribe(features.isBuiltinSpellCheckerEnabled())('spellchecker', function ()
         await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
         await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
 
-    const callWebFrameFn = (expr: string) => w.webContents.executeJavaScript('require("electron").webFrame.' + expr);
+        const callWebFrameFn = (expr: string) => w.webContents.executeJavaScript(`electron.webFrame.${expr}`);
 
         expect(await callWebFrameFn('isWordMisspelled("typography")')).to.equal(false);
         expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(true);
@@ -133,7 +139,7 @@ ifdescribe(features.isBuiltinSpellCheckerEnabled())('spellchecker', function ()
           await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
           await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
 
-      const callWebFrameFn = (expr: string) => w.webContents.executeJavaScript('require("electron").webFrame.' + expr);
+          const callWebFrameFn = (expr: string) => w.webContents.executeJavaScript(`electron.webFrame.${expr}`);
 
           w.webContents.session.spellCheckerEnabled = false;
           v8Util.runUntilIdle();
@@ -225,3 +231,8 @@ ifdescribe(features.isBuiltinSpellCheckerEnabled())('spellchecker', function ()
         });
       });
     });
+  };
+
+  generateSpecs('without sandbox', false);
+  generateSpecs('with sandbox', true);
+});

spec/fixtures/module/preload-electron.js

@@ -0,0 +1 @@
+window.electron = require('electron');