From ae1cef90313a3efe1e9311a850840f677a47ee44 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 1 Nov 2019 17:10:49 -0700 Subject: [PATCH] fix: capture the promise global to avoid userland mutation --- build/webpack/webpack.config.base.js | 7 +++++-- lib/.eslintrc | 8 ++++++++ lib/browser/api/web-contents.js | 4 ++++ lib/browser/desktop-capturer.ts | 4 ++++ lib/browser/devtools.ts | 4 ++++ lib/browser/init.ts | 4 ++++ lib/browser/ipc-main-impl.ts | 4 ++++ lib/browser/ipc-main-internal-utils.ts | 4 ++++ lib/browser/navigation-controller.js | 4 ++++ lib/browser/remote/server.ts | 4 ++++ lib/browser/rpc-server.js | 4 ++++ lib/common/asar.js | 2 ++ lib/common/webpack-globals-provider.ts | 10 ++++++++++ lib/renderer/api/remote.js | 4 ++++ lib/renderer/chrome-api.ts | 4 ++++ spec/api-remote-spec.js | 18 ++++++++++++++++++ spec/static/main.js | 1 + typings/internal-ambient.d.ts | 5 +++++ 18 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 lib/.eslintrc create mode 100644 lib/common/webpack-globals-provider.ts diff --git a/build/webpack/webpack.config.base.js b/build/webpack/webpack.config.base.js index 2afc874ac1f67..6cadd37d1b5b3 100644 --- a/build/webpack/webpack.config.base.js +++ b/build/webpack/webpack.config.base.js @@ -74,7 +74,10 @@ module.exports = ({ global: ['@electron/internal/renderer/webpack-provider', '_global'], Buffer: ['@electron/internal/renderer/webpack-provider', 'Buffer'], }) - ] : []) + ] : []), + new webpack.ProvidePlugin({ + capturedGlobals: ['@electron/internal/common/webpack-globals-provider', 'globals'], + }), ] }) -} \ No newline at end of file +} diff --git a/lib/.eslintrc b/lib/.eslintrc new file mode 100644 index 0000000000000..6a8729e8701e9 --- /dev/null +++ b/lib/.eslintrc @@ -0,0 +1,8 @@ +{ + "rules": { + "no-restricted-globals": ["error", "primordials", "Promise"] + }, + "globals": { + "capturedGlobals": true + } +} diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 5042437e8673f..4c4b8d36006ba 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -11,6 +11,10 @@ const NavigationController = require('@electron/internal/browser/navigation-cont const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal') const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils') +const { + Promise +} = capturedGlobals + // session is not used here, the purpose is to make sure session is initalized // before the webContents module. // eslint-disable-next-line diff --git a/lib/browser/desktop-capturer.ts b/lib/browser/desktop-capturer.ts index 2797b1c9c1ac8..f3c0e50cbedd5 100644 --- a/lib/browser/desktop-capturer.ts +++ b/lib/browser/desktop-capturer.ts @@ -2,6 +2,10 @@ import { EventEmitter } from 'events' const { createDesktopCapturer } = process.electronBinding('desktop_capturer') +const { + Promise +} = capturedGlobals + const deepEqual = (a: ElectronInternal.GetSourcesOptions, b: ElectronInternal.GetSourcesOptions) => JSON.stringify(a) === JSON.stringify(b) let currentlyRunning: { diff --git a/lib/browser/devtools.ts b/lib/browser/devtools.ts index 090ed6be82546..70be08df43ed2 100644 --- a/lib/browser/devtools.ts +++ b/lib/browser/devtools.ts @@ -5,6 +5,10 @@ import * as url from 'url' import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal' import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-utils' +const { + Promise +} = capturedGlobals + const convertToMenuTemplate = function (items: ContextMenuItem[], handler: (id: number) => void) { return items.map(function (item) { const transformed: Electron.MenuItemConstructorOptions = item.type === 'subMenu' ? { diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 47a3924d4b9ee..62d60d2dbe600 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -5,6 +5,10 @@ import * as util from 'util' const Module = require('module') +const { + Promise +} = capturedGlobals + // We modified the original process.argv to let node.js load the init.js, // we need to restore it here. process.argv.splice(1, 1) diff --git a/lib/browser/ipc-main-impl.ts b/lib/browser/ipc-main-impl.ts index 8f3fc9acd6243..79b276ca42501 100644 --- a/lib/browser/ipc-main-impl.ts +++ b/lib/browser/ipc-main-impl.ts @@ -1,6 +1,10 @@ import { EventEmitter } from 'events' import { IpcMainInvokeEvent } from 'electron' +const { + Promise +} = capturedGlobals + export class IpcMainImpl extends EventEmitter { private _invokeHandlers: Map void> = new Map(); diff --git a/lib/browser/ipc-main-internal-utils.ts b/lib/browser/ipc-main-internal-utils.ts index 3c2e4322a74b6..19ac52d081ea4 100644 --- a/lib/browser/ipc-main-internal-utils.ts +++ b/lib/browser/ipc-main-internal-utils.ts @@ -1,5 +1,9 @@ import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal' +const { + Promise +} = capturedGlobals + type IPCHandler = (event: Electron.IpcMainInvokeEvent, ...args: any[]) => any export const handleSync = function (channel: string, handler: T) { diff --git a/lib/browser/navigation-controller.js b/lib/browser/navigation-controller.js index 5176c8ae54102..7e81829aa6f81 100644 --- a/lib/browser/navigation-controller.js +++ b/lib/browser/navigation-controller.js @@ -2,6 +2,10 @@ const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal') +const { + Promise +} = capturedGlobals + // The history operation in renderer is redirected to browser. ipcMainInternal.on('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK', function (event) { event.sender.goBack() diff --git a/lib/browser/remote/server.ts b/lib/browser/remote/server.ts index a622d23027bbe..23628021bb96a 100644 --- a/lib/browser/remote/server.ts +++ b/lib/browser/remote/server.ts @@ -11,6 +11,10 @@ const v8Util = process.electronBinding('v8_util') const eventBinding = process.electronBinding('event') const features = process.electronBinding('features') +const { + Promise +} = capturedGlobals + if (!features.isRemoteModuleEnabled()) { throw new Error('remote module is disabled') } diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 2dd61e05b489f..185ac4abb6e04 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -13,6 +13,10 @@ const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils const guestViewManager = require('@electron/internal/browser/guest-view-manager') const clipboardUtils = require('@electron/internal/common/clipboard-utils') +const { + Promise +} = capturedGlobals + const emitCustomEvent = function (contents, eventName, ...args) { const event = eventBinding.createWithSender(contents) diff --git a/lib/common/asar.js b/lib/common/asar.js index 67b7b813f8515..ebcf1ef939253 100644 --- a/lib/common/asar.js +++ b/lib/common/asar.js @@ -8,6 +8,8 @@ const path = require('path') const util = require('util') + const Promise = global.Promise // eslint-disable-line no-restricted-globals + const envNoAsar = process.env.ELECTRON_NO_ASAR && process.type !== 'browser' && process.type !== 'renderer' diff --git a/lib/common/webpack-globals-provider.ts b/lib/common/webpack-globals-provider.ts new file mode 100644 index 0000000000000..eeaf5237f1544 --- /dev/null +++ b/lib/common/webpack-globals-provider.ts @@ -0,0 +1,10 @@ +// Captures original globals into a scope to ensure that userland modifications do +// not impact Electron. Note that users doing: +// +// global.Promise.resolve = myFn +// +// Will mutate this captured one as well and that is OK. + +export const globals = { + Promise: global.Promise +} diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 53087896d9087..145b6fc76728f 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -7,6 +7,10 @@ const { CallbacksRegistry } = require('@electron/internal/renderer/remote/callba const { isPromise, isSerializableObject } = require('@electron/internal/common/remote/type-utils') const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal') +const { + Promise +} = capturedGlobals + const callbacksRegistry = new CallbacksRegistry() const remoteObjectCache = v8Util.createIDWeakMap() diff --git a/lib/renderer/chrome-api.ts b/lib/renderer/chrome-api.ts index 1f121e5e795f5..fc6b82077498f 100644 --- a/lib/renderer/chrome-api.ts +++ b/lib/renderer/chrome-api.ts @@ -4,6 +4,10 @@ import * as url from 'url' import { Event } from '@electron/internal/renderer/extensions/event' +const { + Promise +} = capturedGlobals + class Tab { public id: number diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index 25ab18f76f0dc..7a3723b012a82 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -517,4 +517,22 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => { const arr = new RUint8Array() }) }) + + describe('with an overriden global Promise constrctor', () => { + let original + + before(() => { + original = Promise + }) + + it('using a promise based method resolves correctly', async () => { + expect(await remote.getGlobal('returnAPromise')(123)).to.equal(123) + global.Promise = { resolve: () => ({}) } + expect(await remote.getGlobal('returnAPromise')(456)).to.equal(456) + }) + + after(() => { + global.Promise = original + }) + }) }) diff --git a/spec/static/main.js b/spec/static/main.js index 624c0a8404ff1..81d850c0558c7 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -75,6 +75,7 @@ ipcMain.on('echo', function (event, msg) { }) global.setTimeoutPromisified = util.promisify(setTimeout) +global.returnAPromise = (value) => new Promise((resolve) => setTimeout(() => resolve(value), 100)) process.removeAllListeners('uncaughtException') process.on('uncaughtException', function (error) { diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 53dabf6f22c9f..bb47ef3097cfa 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -1,4 +1,9 @@ +interface CapturedGlobals { + Promise: typeof Promise; +} + declare var internalBinding: any; +declare var capturedGlobals: CapturedGlobals; declare namespace NodeJS { interface FeaturesBinding {