From 39ce0b5667d65d37b62786d8b9b610bef41adade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 11 May 2023 11:50:59 +0200 Subject: [PATCH 1/2] Remove privileged APIs from the window object --- web/packages/teleterm/src/ui/boot.tsx | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/boot.tsx b/web/packages/teleterm/src/ui/boot.tsx index 67b4d269b4b58..0e8041ed8724a 100644 --- a/web/packages/teleterm/src/ui/boot.tsx +++ b/web/packages/teleterm/src/ui/boot.tsx @@ -47,8 +47,30 @@ async function boot(): Promise { } } +/** + * getElectronGlobals retrieves privileged APIs exposed through the contextBridge from preload.ts. + * It also immediately removes them from the window object so that if an attacker gets to execute + * arbitrary JS on the page, they don't get easy access to those privileged APIs. + * + * Technically, each value exposed through the contextBridge gets frozen. [1] Since we expose a + * promise returning an object however, we can delete properties from that object, effectively + * removing the APIs from the window object. + * + * This behavior might change between Electron or Chromium updates. At the moment we're in the + * process of investigating how brittle this workaround is. [2] + * + * [1] https://www.electronjs.org/docs/latest/api/context-bridge#api + * [2] https://github.com/electron/electron/issues/38243 + */ async function getElectronGlobals(): Promise { - return await window['electron']; + const globals = await window['electron']; + const globalsCopy = { ...globals }; + + for (const property in globals) { + delete globals[property]; + } + + return globalsCopy; } function renderApp(content: React.ReactElement): void { From c3bd97317024a7ae27f45b42b843ddefc4b3ba0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 11 May 2023 11:51:27 +0200 Subject: [PATCH 2/2] Attach app context to window only in dev mode --- .../teleterm/src/ui/appContextProvider.tsx | 8 ++++++-- web/packages/teleterm/src/ui/boot.tsx | 20 +++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/web/packages/teleterm/src/ui/appContextProvider.tsx b/web/packages/teleterm/src/ui/appContextProvider.tsx index b6bcf87b8d00b..d2059d145d276 100644 --- a/web/packages/teleterm/src/ui/appContextProvider.tsx +++ b/web/packages/teleterm/src/ui/appContextProvider.tsx @@ -28,8 +28,12 @@ export default AppContextProvider; export function useAppContext() { const ctx = React.useContext(AppReactContext); - // For debugging and diagnostic purposes. - window['teleterm'] = ctx; + + // Attach the app context to the window for debugging and diagnostic purposes. + // Do not do this in the packaged app as this exposes privileged APIs through the window object. + if (process.env.NODE_ENV === 'development') { + window['teleterm'] = ctx; + } return ctx; } diff --git a/web/packages/teleterm/src/ui/boot.tsx b/web/packages/teleterm/src/ui/boot.tsx index 0e8041ed8724a..4ffc92007c528 100644 --- a/web/packages/teleterm/src/ui/boot.tsx +++ b/web/packages/teleterm/src/ui/boot.tsx @@ -49,23 +49,23 @@ async function boot(): Promise { /** * getElectronGlobals retrieves privileged APIs exposed through the contextBridge from preload.ts. + * * It also immediately removes them from the window object so that if an attacker gets to execute * arbitrary JS on the page, they don't get easy access to those privileged APIs. - * - * Technically, each value exposed through the contextBridge gets frozen. [1] Since we expose a - * promise returning an object however, we can delete properties from that object, effectively - * removing the APIs from the window object. - * - * This behavior might change between Electron or Chromium updates. At the moment we're in the - * process of investigating how brittle this workaround is. [2] - * - * [1] https://www.electronjs.org/docs/latest/api/context-bridge#api - * [2] https://github.com/electron/electron/issues/38243 */ async function getElectronGlobals(): Promise { const globals = await window['electron']; const globalsCopy = { ...globals }; + // Technically, each value exposed through the contextBridge gets frozen. [1] Since we expose a + // promise returning an object however, we can delete properties from that object, effectively + // removing the APIs from the window object. + // + // We suspect that the semantics of this might change between Electron or Chromium updates. + // At the moment we're in the process of investigating how brittle this workaround is. [2] + // + // [1] https://www.electronjs.org/docs/latest/api/context-bridge#api + // [2] https://github.com/electron/electron/issues/38243 for (const property in globals) { delete globals[property]; }