From fe1397e0399c854e460fac5b2c9b17905d9f6b5b Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Wed, 5 Dec 2018 21:57:48 +0100 Subject: [PATCH] security: allow to block desktopCapturer.getSources() calls --- docs/api/app.md | 10 +++++++++ docs/api/web-contents.md | 9 ++++++++ lib/browser/api/web-contents.js | 4 ++++ lib/browser/desktop-capturer.js | 16 ++++++++++++-- lib/renderer/api/desktop-capturer.js | 32 ++++++++++++++++------------ spec/api-app-spec.js | 10 +++++++++ spec/api-desktop-capturer-spec.js | 11 +++++++++- spec/static/main.js | 6 ++++++ 8 files changed, 81 insertions(+), 17 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index 8382944e619eb..8aa59e045aca8 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -398,6 +398,16 @@ non-minimized. This event is guaranteed to be emitted after the `ready` event of `app` gets emitted. +### Event: 'desktop-capturer-get-sources' + +Returns: + +* `event` Event +* `webContents` [WebContents](web-contents.md) + +Emitted when `desktopCapturer.getSources()` is called in the renderer process of `webContents`. +Calling `event.preventDefault()` will make it throw an error. + ### Event: 'remote-require' Returns: diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 0ce2f05c69181..5922e274bd29e 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -663,6 +663,15 @@ Returns: Emitted when the associated window logs a console message. Will not be emitted for windows with *offscreen rendering* enabled. +#### Event: 'desktop-capturer-get-sources' + +Returns: + +* `event` Event + +Emitted when `desktopCapturer.getSources()` is called in the renderer process. +Calling `event.preventDefault()` will make it throw an error. + #### Event: 'remote-require' Returns: diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 2d05b390cf170..d96f60d0eb523 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -368,6 +368,10 @@ WebContents.prototype._init = function () { }) }) + this.on('desktop-capturer-get-sources', (event, ...args) => { + app.emit('desktop-capturer-get-sources', event, this, ...args) + }) + this.on('remote-require', (event, ...args) => { app.emit('remote-require', event, this, ...args) }) diff --git a/lib/browser/desktop-capturer.js b/lib/browser/desktop-capturer.js index c2cdd73ef95cd..9bcb005f321d3 100644 --- a/lib/browser/desktop-capturer.js +++ b/lib/browser/desktop-capturer.js @@ -1,7 +1,10 @@ 'use strict' const ipcMain = require('@electron/internal/browser/ipc-main-internal') +const errorUtils = require('@electron/internal/common/error-utils') + const { desktopCapturer } = process.atomBinding('desktop_capturer') +const eventBinding = process.atomBinding('event') const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b) @@ -12,6 +15,15 @@ const electronSources = 'ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES' const capturerResult = (id) => `ELECTRON_RENDERER_DESKTOP_CAPTURER_RESULT_${id}` ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons, id) => { + const customEvent = eventBinding.createWithSender(event.sender) + event.sender.emit('desktop-capturer-get-sources', customEvent) + + if (customEvent.defaultPrevented) { + const error = new Error('desktopCapturer.getSources() blocked') + event.sender._sendInternal(capturerResult(id), errorUtils.serialize(error)) + return + } + const request = { id, options: { @@ -51,7 +63,7 @@ desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => { }) if (handledWebContents) { - handledWebContents._sendInternal(capturerResult(handledRequest.id), result) + handledWebContents._sendInternal(capturerResult(handledRequest.id), null, result) } // Check the queue to see whether there is another identical request & handle @@ -59,7 +71,7 @@ desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => { const webContents = request.webContents if (deepEqual(handledRequest.options, request.options)) { if (webContents) { - webContents._sendInternal(capturerResult(request.id), result) + webContents._sendInternal(capturerResult(request.id), null, result) } } else { unhandledRequestsQueue.push(request) diff --git a/lib/renderer/api/desktop-capturer.js b/lib/renderer/api/desktop-capturer.js index 74f9e3b3e2617..dcd18f798a02c 100644 --- a/lib/renderer/api/desktop-capturer.js +++ b/lib/renderer/api/desktop-capturer.js @@ -1,7 +1,9 @@ 'use strict' const { nativeImage } = require('electron') + const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal') +const errorUtils = require('@electron/internal/common/error-utils') const includes = [].includes let currentId = 0 @@ -17,6 +19,16 @@ function isValid (options) { return Array.isArray(types) } +function mapSources (sources) { + return sources.map(source => ({ + id: source.id, + name: source.name, + thumbnail: nativeImage.createFromDataURL(source.thumbnail), + display_id: source.display_id, + appIcon: source.appIcon ? nativeImage.createFromDataURL(source.appIcon) : null + })) +} + exports.getSources = function (options, callback) { if (!isValid(options)) return callback(new Error('Invalid options')) const captureWindow = includes.call(options.types, 'window') @@ -34,19 +46,11 @@ exports.getSources = function (options, callback) { const id = incrementId() ipcRenderer.send('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', captureWindow, captureScreen, options.thumbnailSize, options.fetchWindowIcons, id) - return ipcRenderer.once(`ELECTRON_RENDERER_DESKTOP_CAPTURER_RESULT_${id}`, (event, sources) => { - callback(null, (() => { - const results = [] - sources.forEach(source => { - results.push({ - id: source.id, - name: source.name, - thumbnail: nativeImage.createFromDataURL(source.thumbnail), - display_id: source.display_id, - appIcon: source.appIcon ? nativeImage.createFromDataURL(source.appIcon) : null - }) - }) - return results - })()) + return ipcRenderer.once(`ELECTRON_RENDERER_DESKTOP_CAPTURER_RESULT_${id}`, (event, error, sources) => { + if (error) { + callback(errorUtils.deserialize(error)) + } else { + callback(null, mapSources(sources)) + } }) } diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 3a065b4eb1d70..ce3850f2d15bf 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -370,6 +370,16 @@ describe('app module', () => { w = new BrowserWindow({ show: false }) }) + it('should emit desktop-capturer-get-sources event when desktopCapturer.getSources() is invoked', (done) => { + app.once('desktop-capturer-get-sources', (event, webContents) => { + expect(webContents).to.equal(w.webContents) + done() + }) + w = new BrowserWindow({ show: false }) + w.loadURL('about:blank') + w.webContents.executeJavaScript(`require('electron').desktopCapturer.getSources({ types: ['screen'] }, () => {})`) + }) + it('should emit remote-require event when remote.require() is invoked', (done) => { app.once('remote-require', (event, webContents, moduleName) => { expect(webContents).to.equal(w.webContents) diff --git a/spec/api-desktop-capturer-spec.js b/spec/api-desktop-capturer-spec.js index a74a2ac2aacdf..a66c972499661 100644 --- a/spec/api-desktop-capturer-spec.js +++ b/spec/api-desktop-capturer-spec.js @@ -1,6 +1,6 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') -const { desktopCapturer, remote } = require('electron') +const { desktopCapturer, ipcRenderer, remote } = require('electron') const { screen } = remote const features = process.atomBinding('features') @@ -39,6 +39,15 @@ describe('desktopCapturer', () => { }) }) + it('throws an error when blocked', done => { + ipcRenderer.send('handle-next-desktop-capturer-get-sources') + desktopCapturer.getSources({ types: ['screen'] }, (error, sources) => { + expect(error.message).to.equal('desktopCapturer.getSources() blocked') + expect(sources).to.be.undefined() + done() + }) + }) + it('does not throw an error when called more than once (regression)', (done) => { let callCount = 0 const callback = (error, sources) => { diff --git a/spec/static/main.js b/spec/static/main.js index 55bac6bab6ab6..a9d8fc20488a1 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -233,6 +233,12 @@ app.on('ready', function () { }) }) +ipcMain.on('handle-next-desktop-capturer-get-sources', function (event) { + event.sender.once('desktop-capturer-get-sources', (event) => { + event.preventDefault() + }) +}) + ipcMain.on('handle-next-remote-require', function (event, modulesMap = {}) { event.sender.once('remote-require', (event, moduleName) => { event.preventDefault()