From aea8d5325c7a8667153cd292ec13b8de1d53bf3d Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 10 May 2021 14:19:23 +0200 Subject: [PATCH] fix: webFrame spell checker APIs crashing in sandboxed renderers (#29053) --- shell/renderer/api/electron_api_web_frame.cc | 4 +- shell/renderer/electron_renderer_client.cc | 14 +- shell/renderer/electron_renderer_client.h | 4 - shell/renderer/renderer_client_base.cc | 15 +- shell/renderer/renderer_client_base.h | 2 + spec-main/spellchecker-spec.ts | 323 ++++++++++--------- spec/fixtures/module/preload-electron.js | 1 + 7 files changed, 187 insertions(+), 176 deletions(-) create mode 100644 spec/fixtures/module/preload-electron.js diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index 73db4e9063e6..4353d5355be5 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/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(); diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index 6599bc918c10..f35d60e2855c 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/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); diff --git a/shell/renderer/electron_renderer_client.h b/shell/renderer/electron_renderer_client.h index e55bb7b88a1e..57dae75238be 100644 --- a/shell/renderer/electron_renderer_client.h +++ b/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 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 injected_frames_; - static ElectronRendererClient* self_; - DISALLOW_COPY_AND_ASSIGN(ElectronRendererClient); }; diff --git a/shell/renderer/renderer_client_base.cc b/shell/renderer/renderer_client_base.cc index 50c9b8520f65..a0e3fb05d586 100644 --- a/shell/renderer/renderer_client_base.cc +++ b/shell/renderer/renderer_client_base.cc @@ -92,6 +92,9 @@ std::vector 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 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) { diff --git a/shell/renderer/renderer_client_base.h b/shell/renderer/renderer_client_base.h index e0a6d4016cb1..4193ab622f98 100644 --- a/shell/renderer/renderer_client_base.h +++ b/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, diff --git a/spec-main/spellchecker-spec.ts b/spec-main/spellchecker-spec.ts index 63d565ecfa8f..3cc058158729 100644 --- a/spec-main/spellchecker-spec.ts +++ b/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')); - }); + const fixtures = path.resolve(__dirname, '../spec/fixtures'); + const preload = path.join(fixtures, 'module', 'preload-electron.js'); - 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()}`); - }); - - 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); + 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')); }); - it('should return an empty array if no words are added', async () => { - const wordList = await ses.listWordsInSpellCheckerDictionary(); - expect(wordList).to.have.length(0); + 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(`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(`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()}`); + }); + + 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); + }); + }); }); }); + }; - 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); }); diff --git a/spec/fixtures/module/preload-electron.js b/spec/fixtures/module/preload-electron.js new file mode 100644 index 000000000000..9b3482f5df22 --- /dev/null +++ b/spec/fixtures/module/preload-electron.js @@ -0,0 +1 @@ +window.electron = require('electron');