Skip to content

Commit

Permalink
security: allow to block desktopCapturer.getSources() calls (#15964)
Browse files Browse the repository at this point in the history
* security: allow to block desktopCapturer.getSources() calls

* return empty instead of error

* fix: release resources of DesktopCapturer on exit
  • Loading branch information
miniak authored and Cheng Zhao committed Dec 20, 2018
1 parent df0381e commit 547097b
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 16 deletions.
4 changes: 2 additions & 2 deletions atom/browser/api/atom_api_desktop_capturer.h
Expand Up @@ -9,7 +9,7 @@
#include <string>
#include <vector>

#include "atom/browser/api/event_emitter.h"
#include "atom/browser/api/trackable_object.h"
#include "chrome/browser/media/webrtc/desktop_media_list_observer.h"
#include "chrome/browser/media/webrtc/native_desktop_media_list.h"
#include "native_mate/handle.h"
Expand All @@ -18,7 +18,7 @@ namespace atom {

namespace api {

class DesktopCapturer : public mate::EventEmitter<DesktopCapturer>,
class DesktopCapturer : public mate::TrackableObject<DesktopCapturer>,
public DesktopMediaListObserver {
public:
struct Source {
Expand Down
10 changes: 10 additions & 0 deletions docs/api/app.md
Expand Up @@ -401,6 +401,16 @@ gets emitted.
**Note:** Extra command line arguments might be added by Chromium,
such as `--original-process-start-time`.

### 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 return empty sources.

### Event: 'remote-require'

Returns:
Expand Down
9 changes: 9 additions & 0 deletions docs/api/web-contents.md
Expand Up @@ -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 return empty sources.

#### Event: 'remote-require'

Returns:
Expand Down
4 changes: 4 additions & 0 deletions lib/browser/api/web-contents.js
Expand Up @@ -377,6 +377,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)
})
Expand Down
10 changes: 10 additions & 0 deletions lib/browser/desktop-capturer.js
@@ -1,7 +1,9 @@
'use strict'

const ipcMain = require('@electron/internal/browser/ipc-main-internal')

const { desktopCapturer } = process.atomBinding('desktop_capturer')
const eventBinding = process.atomBinding('event')

const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b)

Expand All @@ -12,6 +14,14 @@ 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) {
event.sender._sendInternal(capturerResult(id), [])
return
}

const request = {
id,
options: {
Expand Down
28 changes: 15 additions & 13 deletions lib/renderer/api/desktop-capturer.js
Expand Up @@ -17,6 +17,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')
Expand All @@ -35,18 +45,10 @@ 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
})())
try {
callback(null, mapSources(sources))
} catch (error) {
callback(error)
}
})
}
10 changes: 10 additions & 0 deletions spec/api-app-spec.js
Expand Up @@ -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)
Expand Down
11 changes: 10 additions & 1 deletion 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')

Expand Down Expand Up @@ -99,5 +99,14 @@ describe('desktopCapturer', () => {
}
done()
})

it('returns empty sources when blocked', done => {
ipcRenderer.send('handle-next-desktop-capturer-get-sources')
desktopCapturer.getSources({ types: ['screen'] }, (error, sources) => {
expect(error).to.be.null()
expect(sources).to.be.empty()
done()
})
})
})
})
6 changes: 6 additions & 0 deletions spec/static/main.js
Expand Up @@ -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()
Expand Down

0 comments on commit 547097b

Please sign in to comment.