Permalink
Browse files

fix: security: don't allow arbitrary methods to be invoked on webCont…

…ents via IPC (#15919)
  • Loading branch information...
miniak authored and alexeykuzmin committed Dec 4, 2018
1 parent 0a23c0b commit aa2b2f7c8f3150a5c09b53788eef06a3e710cc4b
@@ -62,6 +62,7 @@ filenames = {
"lib/common/init.js",
"lib/common/parse-features-string.js",
"lib/common/reset-search-paths.js",
"lib/common/web-view-methods.js",
"lib/renderer/callbacks-registry.js",
"lib/renderer/chrome-api.js",
"lib/renderer/content-scripts-injector.js",
@@ -4,6 +4,7 @@ const { webContents } = require('electron')
const ipcMain = require('@electron/internal/browser/ipc-main-internal')
const parseFeaturesString = require('@electron/internal/common/parse-features-string')
const errorUtils = require('@electron/internal/common/error-utils')
const { syncMethods, asyncMethods } = require('@electron/internal/common/web-view-methods')

// Doesn't exist in early initialization.
let webViewManager = null
@@ -368,7 +369,10 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ASYNC_CALL', function (event, request
new Promise(resolve => {
const guest = getGuest(guestInstanceId)
if (guest.hostWebContents !== event.sender) {
throw new Error('Access denied')
throw new Error(`Invalid guestInstanceId: ${guestInstanceId}`)
}
if (!asyncMethods.has(method)) {
throw new Error(`Invalid method: ${method}`)
}
if (hasCallback) {
guest[method](...args, resolve)
@@ -388,9 +392,12 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_SYNC_CALL', function (event, guestIns
try {
const guest = getGuest(guestInstanceId)
if (guest.hostWebContents !== event.sender) {
throw new Error('Access denied')
throw new Error(`Invalid guestInstanceId: ${guestInstanceId}`)
}
if (!syncMethods.has(method)) {
throw new Error(`Invalid method: ${method}`)
}
event.returnValue = [null, guest[method].apply(guest, args)]
event.returnValue = [null, guest[method](...args)]
} catch (error) {
event.returnValue = [errorUtils.serialize(error)]
}
@@ -288,14 +288,19 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSE', function (event, guestI
if (guestWindow != null) guestWindow.destroy()
})

const windowMethods = new Set([
'focus',
'blur'
])

ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_METHOD', function (event, guestId, method, ...args) {
const guestContents = webContents.fromId(guestId)
if (guestContents == null) {
event.returnValue = null
return
}

if (!canAccessWindow(event.sender, guestContents)) {
if (!canAccessWindow(event.sender, guestContents) || !windowMethods.has(method)) {
console.error(`Blocked ${event.sender.getURL()} from calling ${method} on its opener.`)
event.returnValue = null
return
@@ -326,25 +331,35 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_POSTMESSAGE', function (event,
}
})

const webContentsMethods = new Set([
'print',
'executeJavaScript'
])

ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WEB_CONTENTS_METHOD', function (event, guestId, method, ...args) {
const guestContents = webContents.fromId(guestId)
if (guestContents == null) return

if (canAccessWindow(event.sender, guestContents)) {
if (canAccessWindow(event.sender, guestContents) && webContentsMethods.has(method)) {
guestContents[method](...args)
} else {
console.error(`Blocked ${event.sender.getURL()} from calling ${method} on its opener.`)
}
})

const webContentsSyncMethods = new Set([
'getURL',
'loadURL'
])

ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WEB_CONTENTS_METHOD_SYNC', function (event, guestId, method, ...args) {
const guestContents = webContents.fromId(guestId)
if (guestContents == null) {
event.returnValue = null
return
}

if (canAccessWindow(event.sender, guestContents)) {
if (canAccessWindow(event.sender, guestContents) && webContentsSyncMethods.has(method)) {
event.returnValue = guestContents[method](...args)
} else {
console.error(`Blocked ${event.sender.getURL()} from calling ${method} on its opener.`)
@@ -3,12 +3,20 @@
const ipcMain = require('@electron/internal/browser/ipc-main-internal')

// The history operation in renderer is redirected to browser.
ipcMain.on('ELECTRON_NAVIGATION_CONTROLLER', function (event, method, ...args) {
event.sender[method](...args)
ipcMain.on('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK', function (event) {
event.sender.goBack()
})

ipcMain.on('ELECTRON_SYNC_NAVIGATION_CONTROLLER', function (event, method, ...args) {
event.returnValue = event.sender[method](...args)
ipcMain.on('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD', function (event) {
event.sender.goForward()
})

ipcMain.on('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', function (event, offset) {
event.sender.goToOffset(offset)
})

ipcMain.on('ELECTRON_NAVIGATION_CONTROLLER_LENGTH', function (event) {
event.returnValue = event.sender.length()
})

// JavaScript implementation of Chromium's NavigationController.
@@ -0,0 +1,67 @@
'use strict'

// Public-facing API methods.
exports.syncMethods = new Set([
'getURL',
'loadURL',
'getTitle',
'isLoading',
'isLoadingMainFrame',
'isWaitingForResponse',
'stop',
'reload',
'reloadIgnoringCache',
'canGoBack',
'canGoForward',
'canGoToOffset',
'clearHistory',
'goBack',
'goForward',
'goToIndex',
'goToOffset',
'isCrashed',
'setUserAgent',
'getUserAgent',
'openDevTools',
'closeDevTools',
'isDevToolsOpened',
'isDevToolsFocused',
'inspectElement',
'setAudioMuted',
'isAudioMuted',
'isCurrentlyAudible',
'undo',
'redo',
'cut',
'copy',
'paste',
'pasteAndMatchStyle',
'delete',
'selectAll',
'unselect',
'replace',
'replaceMisspelling',
'findInPage',
'stopFindInPage',
'downloadURL',
'inspectServiceWorker',
'showDefinitionForSelection',
'setZoomFactor',
'setZoomLevel'
])

exports.asyncMethods = new Set([
'insertCSS',
'insertText',
'send',
'sendInputEvent',
'setLayoutZoomLevelLimits',
'setVisualZoomLevelLimits',
// with callback
'capturePage',
'executeJavaScript',
'getZoomFactor',
'getZoomLevel',
'print',
'printToPDF'
])
@@ -7,6 +7,7 @@ const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
const guestViewInternal = require('@electron/internal/renderer/web-view/guest-view-internal')
const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants')
const errorUtils = require('@electron/internal/common/error-utils')
const { syncMethods, asyncMethods } = require('@electron/internal/common/web-view-methods')

// ID generator.
let nextId = 0
@@ -230,71 +231,6 @@ const registerWebViewElement = function () {
}
}

// Public-facing API methods.
const methods = [
'getURL',
'loadURL',
'getTitle',
'isLoading',
'isLoadingMainFrame',
'isWaitingForResponse',
'stop',
'reload',
'reloadIgnoringCache',
'canGoBack',
'canGoForward',
'canGoToOffset',
'clearHistory',
'goBack',
'goForward',
'goToIndex',
'goToOffset',
'isCrashed',
'setUserAgent',
'getUserAgent',
'openDevTools',
'closeDevTools',
'isDevToolsOpened',
'isDevToolsFocused',
'inspectElement',
'setAudioMuted',
'isAudioMuted',
'isCurrentlyAudible',
'undo',
'redo',
'cut',
'copy',
'paste',
'pasteAndMatchStyle',
'delete',
'selectAll',
'unselect',
'replace',
'replaceMisspelling',
'findInPage',
'stopFindInPage',
'downloadURL',
'inspectServiceWorker',
'showDefinitionForSelection',
'setZoomFactor',
'setZoomLevel'
]
const nonblockMethods = [
'insertCSS',
'insertText',
'send',
'sendInputEvent',
'setLayoutZoomLevelLimits',
'setVisualZoomLevelLimits',
// with callback
'capturePage',
'executeJavaScript',
'getZoomFactor',
'getZoomLevel',
'print',
'printToPDF'
]

const getGuestInstanceId = function (self) {
const internal = v8Util.getHiddenValue(self, 'internal')
if (!internal.guestInstanceId) {
@@ -314,7 +250,7 @@ const registerWebViewElement = function () {
}
}
}
for (const method of methods) {
for (const method of syncMethods) {
proto[method] = createBlockHandler(method)
}

@@ -332,7 +268,7 @@ const registerWebViewElement = function () {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_ASYNC_CALL', requestId, getGuestInstanceId(this), method, args, callback != null)
}
}
for (const method of nonblockMethods) {
for (const method of asyncMethods) {
proto[method] = createNonBlockHandler(method)
}

@@ -146,15 +146,6 @@ function BrowserWindowProxy (ipcRenderer, guestId) {
}
}

// Forward history operations to browser.
const sendHistoryOperation = function (ipcRenderer, ...args) {
ipcRenderer.send('ELECTRON_NAVIGATION_CONTROLLER', ...args)
}

const getHistoryOperation = function (ipcRenderer, ...args) {
return ipcRenderer.sendSync('ELECTRON_SYNC_NAVIGATION_CONTROLLER', ...args)
}

module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNativeWindowOpen) => {
if (guestInstanceId == null) {
// Override default window.close.
@@ -199,20 +190,20 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative
})

window.history.back = function () {
sendHistoryOperation(ipcRenderer, 'goBack')
ipcRenderer.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK')
}

window.history.forward = function () {
sendHistoryOperation(ipcRenderer, 'goForward')
ipcRenderer.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD')
}

window.history.go = function (offset) {
sendHistoryOperation(ipcRenderer, 'goToOffset', +offset)
ipcRenderer.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset)
}

defineProperty(window.history, 'length', {
get: function () {
return getHistoryOperation(ipcRenderer, 'length')
return ipcRenderer.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH')
}
})

0 comments on commit aa2b2f7

Please sign in to comment.