Browse Source

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
Samuel Attard 1 year ago
parent
commit
83892ab995

+ 13 - 0
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.

+ 21 - 21
lib/renderer/api/ipc-renderer.ts

@@ -3,30 +3,30 @@ import { EventEmitter } from 'events';
 const { ipc } = process._linkedBinding('electron_renderer_ipc');
 
 const internal = false;
+class IpcRenderer extends EventEmitter implements Electron.IpcRenderer {
+  send (channel: string, ...args: any[]) {
+    return ipc.send(internal, channel, args);
+  }
 
-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);
-};
+  sendSync (channel: string, ...args: any[]) {
+    return ipc.sendSync(internal, channel, args);
+  }
 
-ipcRenderer.sendToHost = function (channel, ...args) {
-  return ipc.sendToHost(channel, args);
-};
+  sendToHost (channel: string, ...args: any[]) {
+    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}`);
+  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;
   }
-  return result;
-};
 
-ipcRenderer.postMessage = function (channel: string, message: any, transferables: any) {
-  return ipc.postMessage(channel, message, transferables);
-};
+  postMessage (channel: string, message: any, transferables: any) {
+    return ipc.postMessage(channel, message, transferables);
+  }
+}
 
-export default ipcRenderer;
+export default new IpcRenderer();

+ 16 - 14
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;
+class IpcRendererInternal extends EventEmitter implements ElectronInternal.IpcRendererInternal {
+  send (channel: string, ...args: any[]) {
+    return ipc.send(internal, channel, args);
+  }
 
-ipcRendererInternal.send = function (channel, ...args) {
-  return ipc.send(internal, channel, args);
-};
+  sendSync (channel: string, ...args: any[]) {
+    return ipc.sendSync(internal, channel, args);
+  }
 
-ipcRendererInternal.sendSync = function (channel, ...args) {
-  return ipc.sendSync(internal, channel, args);
-};
+  async invoke<T> (channel: string, ...args: any[]) {
+    const { error, result } = await ipc.invoke<T>(internal, channel, args);
+    if (error) {
+      throw new Error(`Error invoking remote method '${channel}': ${error}`);
+    }
+    return result;
+  };
+}
 
-ipcRendererInternal.invoke = async function<T> (channel: string, ...args: any[]) {
-  const { error, result } = await ipc.invoke<T>(internal, channel, args);
-  if (error) {
-    throw new Error(`Error invoking remote method '${channel}': ${error}`);
-  }
-  return result;
-};
+export const ipcRendererInternal = new IpcRendererInternal();

+ 2 - 1
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++) {

+ 2 - 2
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);