From 83892ab995fb7a0b60ebc0c8837b2dba35ec5262 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 31 Oct 2023 14:29:40 -0700 Subject: [PATCH] refactor: ensure IpcRenderer is not bridgable (#40330) * refactor: ensure IpcRenderer is not bridgable * chore: add notes to breaking-changes * spec: fix test that bridged ipcrenderer --- docs/breaking-changes.md | 13 ++++++ lib/renderer/api/ipc-renderer.ts | 48 +++++++++++------------ lib/renderer/ipc-renderer-internal.ts | 34 ++++++++-------- spec/fixtures/apps/libuv-hang/preload.js | 3 +- spec/fixtures/apps/libuv-hang/renderer.js | 4 +- 5 files changed, 59 insertions(+), 43 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index ddffa5f081..682f1bb554 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -14,6 +14,19 @@ This document uses the following convention to categorize breaking changes: ## Planned Breaking API Changes (29.0) +### Behavior Changed: `ipcRenderer` can no longer be sent over the `contextBridge` + +Attempting to send `ipcRenderer` as an object over the `contextBridge` will now result in +an empty object on the receiving side of the bridge. This change was made to remove / mitigate +a security footgun, you should not directly expose ipcRenderer or it's methods over the bridge. +Instead provide a safe wrapper like below: + +```js +contextBridge.exposeInMainWorld('app', { + onEvent: (cb) => ipcRenderer.on('foo', (e, ...args) => cb(args)) +}) +``` + ### Removed: `renderer-process-crashed` event on `app` The `renderer-process-crashed` event on `app` has been removed. diff --git a/lib/renderer/api/ipc-renderer.ts b/lib/renderer/api/ipc-renderer.ts index b637f19271..c82f862123 100644 --- a/lib/renderer/api/ipc-renderer.ts +++ b/lib/renderer/api/ipc-renderer.ts @@ -3,30 +3,30 @@ import { EventEmitter } from 'events'; const { ipc } = process._linkedBinding('electron_renderer_ipc'); const internal = false; - -const ipcRenderer = new EventEmitter() as Electron.IpcRenderer; -ipcRenderer.send = function (channel, ...args) { - return ipc.send(internal, channel, args); -}; - -ipcRenderer.sendSync = function (channel, ...args) { - return ipc.sendSync(internal, channel, args); -}; - -ipcRenderer.sendToHost = function (channel, ...args) { - return ipc.sendToHost(channel, args); -}; - -ipcRenderer.invoke = async function (channel, ...args) { - const { error, result } = await ipc.invoke(internal, channel, args); - if (error) { - throw new Error(`Error invoking remote method '${channel}': ${error}`); +class IpcRenderer extends EventEmitter implements Electron.IpcRenderer { + send (channel: string, ...args: any[]) { + return ipc.send(internal, channel, args); } - return result; -}; -ipcRenderer.postMessage = function (channel: string, message: any, transferables: any) { - return ipc.postMessage(channel, message, transferables); -}; + sendSync (channel: string, ...args: any[]) { + return ipc.sendSync(internal, channel, args); + } -export default ipcRenderer; + sendToHost (channel: string, ...args: any[]) { + return ipc.sendToHost(channel, args); + } + + async invoke (channel: string, ...args: any[]) { + const { error, result } = await ipc.invoke(internal, channel, args); + if (error) { + throw new Error(`Error invoking remote method '${channel}': ${error}`); + } + return result; + } + + postMessage (channel: string, message: any, transferables: any) { + return ipc.postMessage(channel, message, transferables); + } +} + +export default new IpcRenderer(); diff --git a/lib/renderer/ipc-renderer-internal.ts b/lib/renderer/ipc-renderer-internal.ts index f37ea0d724..da832d2e47 100644 --- a/lib/renderer/ipc-renderer-internal.ts +++ b/lib/renderer/ipc-renderer-internal.ts @@ -4,20 +4,22 @@ const { ipc } = process._linkedBinding('electron_renderer_ipc'); const internal = true; -export const ipcRendererInternal = new EventEmitter() as any as ElectronInternal.IpcRendererInternal; - -ipcRendererInternal.send = function (channel, ...args) { - return ipc.send(internal, channel, args); -}; - -ipcRendererInternal.sendSync = function (channel, ...args) { - return ipc.sendSync(internal, channel, args); -}; - -ipcRendererInternal.invoke = async function (channel: string, ...args: any[]) { - const { error, result } = await ipc.invoke(internal, channel, args); - if (error) { - throw new Error(`Error invoking remote method '${channel}': ${error}`); +class IpcRendererInternal extends EventEmitter implements ElectronInternal.IpcRendererInternal { + send (channel: string, ...args: any[]) { + return ipc.send(internal, channel, args); } - return result; -}; + + sendSync (channel: string, ...args: any[]) { + return ipc.sendSync(internal, channel, args); + } + + async invoke (channel: string, ...args: any[]) { + const { error, result } = await ipc.invoke(internal, channel, args); + if (error) { + throw new Error(`Error invoking remote method '${channel}': ${error}`); + } + return result; + }; +} + +export const ipcRendererInternal = new IpcRendererInternal(); diff --git a/spec/fixtures/apps/libuv-hang/preload.js b/spec/fixtures/apps/libuv-hang/preload.js index 2629549fbf..d788aec429 100644 --- a/spec/fixtures/apps/libuv-hang/preload.js +++ b/spec/fixtures/apps/libuv-hang/preload.js @@ -1,7 +1,8 @@ const { contextBridge, ipcRenderer } = require('electron'); contextBridge.exposeInMainWorld('api', { - ipcRenderer, + // This is not safe, do not copy this code into your app + invoke: (...args) => ipcRenderer.invoke(...args), run: async () => { const { promises: fs } = require('node:fs'); for (let i = 0; i < 10; i++) { diff --git a/spec/fixtures/apps/libuv-hang/renderer.js b/spec/fixtures/apps/libuv-hang/renderer.js index cdbc4aab4d..eb822ca519 100644 --- a/spec/fixtures/apps/libuv-hang/renderer.js +++ b/spec/fixtures/apps/libuv-hang/renderer.js @@ -1,6 +1,6 @@ -const { run, ipcRenderer } = window.api; +const { run, invoke } = window.api; run().then(async () => { - const count = await ipcRenderer.invoke('reload-successful'); + const count = await invoke('reload-successful'); if (count < 3) location.reload(); }).catch(console.log);