diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index fb7fd8987c..d4c30950ef 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -412,6 +412,7 @@ import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-market import { createWakeIndicatorWindowController } from './wake-indicator-window' import { enumerateWindowsFrontToBack, enumerationFailed, readWindowBelow } from './window-below' import { registrySshScopeForWindowRoute, WindowConnectionRouteRegistry } from './window-connection-route' +import { createWindowOpenHandler } from './window-open-policy' import { installWindowRendererLifecycle } from './window-renderer-lifecycle' import { createWindowRevealController } from './window-reveal' import { @@ -13242,11 +13243,17 @@ function wireCommonWindowHandlers(win, { zoom = true }: { zoom?: boolean } = {}) } installContextMenuBridge(win) - win.webContents.setWindowOpenHandler(details => { - openExternalUrl(details.url) - - return { action: 'deny' } - }) + // Deny every window-open request and NEVER open a URL as a side effect here. + // Trusted external links go through the audited `hermes:openExternal` IPC + // channel; the only content that reaches this handler is what we did not + // initiate — including untrusted HTML in sandboxed `allow-scripts` iframes. + // Opening `details.url` here is the GHSA-9f4c-93c8-jc8g (CVE-2026-70608) + // vector: a sandboxed iframe with no `allow-popups` and no user gesture can + // force the OS browser to an attacker URL. No fixed 40.x Electron exists, so + // we close it at the seam. See electron/window-open-policy.ts. + win.webContents.setWindowOpenHandler( + createWindowOpenHandler(url => rememberLog(`[window-open] denied: ${url}`)) + ) win.webContents.on('will-navigate', (event, url) => { if ((DEV_SERVER && url.startsWith(DEV_SERVER)) || (!DEV_SERVER && url.startsWith('file:'))) { return diff --git a/apps/desktop/electron/window-open-policy.ts b/apps/desktop/electron/window-open-policy.ts new file mode 100644 index 0000000000..cf8304ea90 --- /dev/null +++ b/apps/desktop/electron/window-open-policy.ts @@ -0,0 +1,55 @@ +/** + * Window-open policy for every BrowserWindow's webContents. + * + * In the Electron desktop app, every external URL we open on purpose is routed + * through the audited `hermes:openExternal` IPC channel (see `openExternalUrl` + * in main.ts, which enforces an http/https/mailto scheme allowlist and guards + * file: through the IPC path resolver). The `window.open` / `target=_blank` + * path that reaches `setWindowOpenHandler` is therefore, inside Electron, only + * ever driven by content we did NOT initiate — most dangerously untrusted HTML + * rendered in sandboxed `allow-scripts` iframes (artifact previews). + * + * GHSA-9f4c-93c8-jc8g (CVE-2026-70608, High 7.2) lets such a sandboxed iframe + * reach this handler with NO user interaction and WITHOUT `allow-popups`. If + * the handler opens `details.url` as a side effect, a malicious artifact can + * force the user's real browser to an attacker-chosen URL. There is no fixed + * 40.x Electron release (the fix is 41.10.3+/42.0.1), so we defend at the seam + * regardless of Electron version: deny every window-open request and NEVER open + * a URL as a side effect here. Trusted opens keep working because they go + * through the IPC channel, not this handler. + */ + +export interface WindowOpenRequestLike { + url: string +} + +export interface WindowOpenDecision { + action: 'deny' +} + +/** + * The security decision for a window-open request. Always deny — see the module + * comment. Kept as a named function so the contract has one tested home and a + * future edit that tries to reintroduce conditional opening has to defeat the + * test rather than silently succeed. + */ +export function decideWindowOpen(_request: WindowOpenRequestLike): WindowOpenDecision { + return { action: 'deny' } +} + +/** + * Build a `setWindowOpenHandler` callback. It denies unconditionally and never + * opens anything. `onDenied` is an optional observability hook (logging only); + * it MUST NOT open a URL — doing so would re-open the CVE this handler closes. + */ +export function createWindowOpenHandler( + onDenied?: (url: string) => void +): (details: WindowOpenRequestLike) => WindowOpenDecision { + return details => { + if (onDenied) { + onDenied(details.url) + } + + return decideWindowOpen(details) + } +} diff --git a/apps/desktop/src/app/settings/env-var-actions-menu.tsx b/apps/desktop/src/app/settings/env-var-actions-menu.tsx index fadab259f6..6fcee9d5a0 100644 --- a/apps/desktop/src/app/settings/env-var-actions-menu.tsx +++ b/apps/desktop/src/app/settings/env-var-actions-menu.tsx @@ -10,6 +10,7 @@ import { import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' import { useI18n } from '@/i18n' +import { openExternalLink } from '@/lib/external-link' import { triggerHaptic } from '@/lib/haptics' import { ExternalLink, Eye, EyeOff, KeyRound, Trash2 } from '@/lib/icons' import { cn } from '@/lib/utils' @@ -61,7 +62,7 @@ function useEnvVarItems({ onSelect: event => { event.preventDefault() triggerHaptic('selection') - window.open(docsUrl!, '_blank', 'noopener,noreferrer') + openExternalLink(docsUrl!) } }) } diff --git a/tests-js/window-open-policy.test.ts b/tests-js/window-open-policy.test.ts new file mode 100644 index 0000000000..3c029ecdc2 --- /dev/null +++ b/tests-js/window-open-policy.test.ts @@ -0,0 +1,67 @@ +/** + * Security regression for GHSA-9f4c-93c8-jc8g (CVE-2026-70608, High 7.2): + * "Sandboxed iframe can bypass the allow-popups restriction via the OpenURL + * navigation path." + * + * A sandboxed iframe without `allow-popups` can reach a window's + * `setWindowOpenHandler` with no user gesture. The desktop app renders + * untrusted artifact HTML in `