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

shell/renderer/api/electron_api_web_frame.cc

@@ -29,7 +29,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"
@@ -118,7 +118,7 @@ bool SpellCheckWord(v8::Isolate* isolate,
   size_t start;
   size_t length;
 
-  ElectronRendererClient* client = ElectronRendererClient::Get();
+  RendererClientBase* client = RendererClientBase::Get();
   auto* render_frame = GetRenderFrame(window);
   if (!render_frame)
     return true;

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())) {
-  DCHECK(!self_) << "Cannot have two ElectronRendererClient";
-  self_ = this;
-}
+      electron_bindings_(new ElectronBindings(node_bindings_->uv_loop())) {}
 
 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,166 +62,177 @@ ifdescribe(features.isBuiltinSpellCheckerEnabled())('spellchecker', function ()
   });
   after(() => server.close());
 
-  beforeEach(async () => {
-    w = new BrowserWindow({
-      show: false,
-      webPreferences: {
-        nodeIntegration: true,
-        partition: `unique-spell-${Date.now()}`,
-        contextIsolation: false
-      }
-    });
-    w.webContents.session.setSpellCheckerDictionaryDownloadURL(`http://127.0.0.1:${(server.address() as AddressInfo).port}/`);
-    w.webContents.session.setSpellCheckerLanguages(['en-US']);
-    await w.loadFile(path.resolve(__dirname, './fixtures/chromium/spellchecker.html'));
-  });
-
-  afterEach(async () => {
-    await closeWindow(w);
-  });
-
-  // Context menu test can not run on Windows.
-  const shouldRun = process.platform !== 'win32';
-
-  ifit(shouldRun)('should detect correctly spelled words as correct', async () => {
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typography"');
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
-    const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.selectionText.length > 0);
-    expect(contextMenuParams.misspelledWord).to.eq('');
-    expect(contextMenuParams.dictionarySuggestions).to.have.lengthOf(0);
-  });
-
-  ifit(shouldRun)('should detect incorrectly spelled words as incorrect', async () => {
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
-    const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
-    expect(contextMenuParams.misspelledWord).to.eq('typograpy');
-    expect(contextMenuParams.dictionarySuggestions).to.have.length.of.at.least(1);
-  });
-
-  ifit(shouldRun)('should detect incorrectly spelled words as incorrect after disabling all languages and re-enabling', async () => {
-    w.webContents.session.setSpellCheckerLanguages([]);
-    await delay(500);
-    w.webContents.session.setSpellCheckerLanguages(['en-US']);
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
-    const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
-    expect(contextMenuParams.misspelledWord).to.eq('typograpy');
-    expect(contextMenuParams.dictionarySuggestions).to.have.length.of.at.least(1);
-  });
-
-  ifit(shouldRun)('should expose webFrame spellchecker correctly', async () => {
-    await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
-    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);
-
-    expect(await callWebFrameFn('isWordMisspelled("typography")')).to.equal(false);
-    expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(true);
-    expect(await callWebFrameFn('getWordSuggestions("typography")')).to.be.empty();
-    expect(await callWebFrameFn('getWordSuggestions("typograpy")')).to.not.be.empty();
-  });
-
-  describe('spellCheckerEnabled', () => {
-    it('is enabled by default', async () => {
-      expect(w.webContents.session.spellCheckerEnabled).to.be.true();
-    });
-
-    ifit(shouldRun)('can be dynamically changed', async () => {
-      await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
-      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);
-
-      w.webContents.session.spellCheckerEnabled = false;
-      v8Util.runUntilIdle();
-      expect(w.webContents.session.spellCheckerEnabled).to.be.false();
-      // spellCheckerEnabled is sent to renderer asynchronously and there is
-      // no event notifying when it is finished, so wait a little while to
-      // ensure the setting has been changed in renderer.
-      await delay(500);
-      expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(false);
-
-      w.webContents.session.spellCheckerEnabled = true;
-      v8Util.runUntilIdle();
-      expect(w.webContents.session.spellCheckerEnabled).to.be.true();
-      await delay(500);
-      expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(true);
-    });
-  });
-
-  describe('custom dictionary word list API', () => {
-    let ses: Session;
-
-    beforeEach(async () => {
-      // ensure a new session runs on each test run
-      ses = session.fromPartition(`persist:customdictionary-test-${Date.now()}`);
-    });
+  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: {
+            partition: `unique-spell-${Date.now()}`,
+            contextIsolation: false,
+            preload,
+            sandbox
+          }
+        });
+        w.webContents.session.setSpellCheckerDictionaryDownloadURL(`http://127.0.0.1:${(server.address() as AddressInfo).port}/`);
+        w.webContents.session.setSpellCheckerLanguages(['en-US']);
+        await w.loadFile(path.resolve(__dirname, './fixtures/chromium/spellchecker.html'));
+      });
 
-    afterEach(async () => {
-      if (ses) {
-        await ses.clearStorageData();
-        ses = null as any;
-      }
-    });
+      afterEach(async () => {
+        await closeWindow(w);
+      });
 
-    describe('ses.listWordsFromSpellCheckerDictionary', () => {
-      it('should successfully list words in custom dictionary', async () => {
-        const words = ['foo', 'bar', 'baz'];
-        const results = words.map(word => ses.addWordToSpellCheckerDictionary(word));
-        expect(results).to.eql([true, true, true]);
+      // Context menu test can not run on Windows.
+      const shouldRun = process.platform !== 'win32';
 
-        const wordList = await ses.listWordsInSpellCheckerDictionary();
-        expect(wordList).to.have.deep.members(words);
+      ifit(shouldRun)('should detect correctly spelled words as correct', async () => {
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typography"');
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
+        const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.selectionText.length > 0);
+        expect(contextMenuParams.misspelledWord).to.eq('');
+        expect(contextMenuParams.dictionarySuggestions).to.have.lengthOf(0);
       });
 
-      it('should return an empty array if no words are added', async () => {
-        const wordList = await ses.listWordsInSpellCheckerDictionary();
-        expect(wordList).to.have.length(0);
+      ifit(shouldRun)('should detect incorrectly spelled words as incorrect', async () => {
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
+        const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
+        expect(contextMenuParams.misspelledWord).to.eq('typograpy');
+        expect(contextMenuParams.dictionarySuggestions).to.have.length.of.at.least(1);
       });
-    });
 
-    describe('ses.addWordToSpellCheckerDictionary', () => {
-      it('should successfully add word to custom dictionary', async () => {
-        const result = ses.addWordToSpellCheckerDictionary('foobar');
-        expect(result).to.equal(true);
-        const wordList = await ses.listWordsInSpellCheckerDictionary();
-        expect(wordList).to.eql(['foobar']);
+      ifit(shouldRun)('should detect incorrectly spelled words as incorrect after disabling all languages and re-enabling', async () => {
+        w.webContents.session.setSpellCheckerLanguages([]);
+        await delay(500);
+        w.webContents.session.setSpellCheckerLanguages(['en-US']);
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
+        const contextMenuParams = await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
+        expect(contextMenuParams.misspelledWord).to.eq('typograpy');
+        expect(contextMenuParams.dictionarySuggestions).to.have.length.of.at.least(1);
       });
 
-      it('should fail for an empty string', async () => {
-        const result = ses.addWordToSpellCheckerDictionary('');
-        expect(result).to.equal(false);
-        const wordList = await ses.listWordsInSpellCheckerDictionary;
-        expect(wordList).to.have.length(0);
-      });
+      ifit(shouldRun)('should expose webFrame spellchecker correctly', async () => {
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
+        await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
+        await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
 
-      // remove API will always return false because we can't add words
-      it('should fail for non-persistent sessions', async () => {
-        const tempSes = session.fromPartition('temporary');
-        const result = tempSes.addWordToSpellCheckerDictionary('foobar');
-        expect(result).to.equal(false);
+        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);
+        expect(await callWebFrameFn('getWordSuggestions("typography")')).to.be.empty();
+        expect(await callWebFrameFn('getWordSuggestions("typograpy")')).to.not.be.empty();
       });
-    });
 
-    describe('ses.removeWordFromSpellCheckerDictionary', () => {
-      it('should successfully remove words to custom dictionary', async () => {
-        const result1 = ses.addWordToSpellCheckerDictionary('foobar');
-        expect(result1).to.equal(true);
-        const wordList1 = await ses.listWordsInSpellCheckerDictionary();
-        expect(wordList1).to.eql(['foobar']);
-        const result2 = ses.removeWordFromSpellCheckerDictionary('foobar');
-        expect(result2).to.equal(true);
-        const wordList2 = await ses.listWordsInSpellCheckerDictionary();
-        expect(wordList2).to.have.length(0);
+      describe('spellCheckerEnabled', () => {
+        it('is enabled by default', async () => {
+          expect(w.webContents.session.spellCheckerEnabled).to.be.true();
+        });
+
+        ifit(shouldRun)('can be dynamically changed', async () => {
+          await w.webContents.executeJavaScript('document.body.querySelector("textarea").value = "typograpy"');
+          await w.webContents.executeJavaScript('document.body.querySelector("textarea").focus()');
+          await rightClickUntil((contextMenuParams) => contextMenuParams.misspelledWord.length > 0);
+
+          const callWebFrameFn = (expr: string) => w.webContents.executeJavaScript(`electron.webFrame.${expr}`);
+
+          w.webContents.session.spellCheckerEnabled = false;
+          v8Util.runUntilIdle();
+          expect(w.webContents.session.spellCheckerEnabled).to.be.false();
+          // spellCheckerEnabled is sent to renderer asynchronously and there is
+          // no event notifying when it is finished, so wait a little while to
+          // ensure the setting has been changed in renderer.
+          await delay(500);
+          expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(false);
+
+          w.webContents.session.spellCheckerEnabled = true;
+          v8Util.runUntilIdle();
+          expect(w.webContents.session.spellCheckerEnabled).to.be.true();
+          await delay(500);
+          expect(await callWebFrameFn('isWordMisspelled("typograpy")')).to.equal(true);
+        });
       });
 
-      it('should fail for words not in custom dictionary', () => {
-        const result2 = ses.removeWordFromSpellCheckerDictionary('foobar');
-        expect(result2).to.equal(false);
+      describe('custom dictionary word list API', () => {
+        let ses: Session;
+
+        beforeEach(async () => {
+          // ensure a new session runs on each test run
+          ses = session.fromPartition(`persist:customdictionary-test-${Date.now()}`);
+        });
+
+        afterEach(async () => {
+          if (ses) {
+            await ses.clearStorageData();
+            ses = null as any;
+          }
+        });
+
+        describe('ses.listWordsFromSpellCheckerDictionary', () => {
+          it('should successfully list words in custom dictionary', async () => {
+            const words = ['foo', 'bar', 'baz'];
+            const results = words.map(word => ses.addWordToSpellCheckerDictionary(word));
+            expect(results).to.eql([true, true, true]);
+
+            const wordList = await ses.listWordsInSpellCheckerDictionary();
+            expect(wordList).to.have.deep.members(words);
+          });
+
+          it('should return an empty array if no words are added', async () => {
+            const wordList = await ses.listWordsInSpellCheckerDictionary();
+            expect(wordList).to.have.length(0);
+          });
+        });
+
+        describe('ses.addWordToSpellCheckerDictionary', () => {
+          it('should successfully add word to custom dictionary', async () => {
+            const result = ses.addWordToSpellCheckerDictionary('foobar');
+            expect(result).to.equal(true);
+            const wordList = await ses.listWordsInSpellCheckerDictionary();
+            expect(wordList).to.eql(['foobar']);
+          });
+
+          it('should fail for an empty string', async () => {
+            const result = ses.addWordToSpellCheckerDictionary('');
+            expect(result).to.equal(false);
+            const wordList = await ses.listWordsInSpellCheckerDictionary;
+            expect(wordList).to.have.length(0);
+          });
+
+          // remove API will always return false because we can't add words
+          it('should fail for non-persistent sessions', async () => {
+            const tempSes = session.fromPartition('temporary');
+            const result = tempSes.addWordToSpellCheckerDictionary('foobar');
+            expect(result).to.equal(false);
+          });
+        });
+
+        describe('ses.removeWordFromSpellCheckerDictionary', () => {
+          it('should successfully remove words to custom dictionary', async () => {
+            const result1 = ses.addWordToSpellCheckerDictionary('foobar');
+            expect(result1).to.equal(true);
+            const wordList1 = await ses.listWordsInSpellCheckerDictionary();
+            expect(wordList1).to.eql(['foobar']);
+            const result2 = ses.removeWordFromSpellCheckerDictionary('foobar');
+            expect(result2).to.equal(true);
+            const wordList2 = await ses.listWordsInSpellCheckerDictionary();
+            expect(wordList2).to.have.length(0);
+          });
+
+          it('should fail for words not in custom dictionary', () => {
+            const result2 = ses.removeWordFromSpellCheckerDictionary('foobar');
+            expect(result2).to.equal(false);
+          });
+        });
       });
     });
-  });
+  };
+
+  generateSpecs('without sandbox', false);
+  generateSpecs('with sandbox', true);
 });

spec/fixtures/module/preload-electron.js

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