From 4eff3dc09e4d1e62d649c5ce9902f532bb7469c7 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Sat, 11 Apr 2026 09:12:17 -0500 Subject: [PATCH] fix: restrict window.open features to allowlisted BrowserWindow options (#50946) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Sam Attard --- docs/api/window-open.md | 19 +++++++----- lib/browser/parse-features-string.ts | 30 ++++++++++++++++++- spec/guest-window-manager-spec.ts | 26 ++++++++++++++++ spec/parse-features-string-spec.ts | 44 +++++++++++++++++++++++++++- 4 files changed, 110 insertions(+), 9 deletions(-) diff --git a/docs/api/window-open.md b/docs/api/window-open.md index e9c4b80550..75f746f2c3 100644 --- a/docs/api/window-open.md +++ b/docs/api/window-open.md @@ -33,10 +33,14 @@ because it is invoked in the main process. Returns [`Window`](https://developer.mozilla.org/en-US/docs/Web/API/Window) | null `features` is a comma-separated key-value list, following the standard format of -the browser. Electron will parse [`BrowserWindowConstructorOptions`](structures/browser-window-options.md) out of this -list where possible, for convenience. For full control and better ergonomics, -consider using `webContents.setWindowOpenHandler` to customize the -BrowserWindow creation. +the browser. For convenience, Electron will parse a subset of presentational +[`BrowserWindowConstructorOptions`](structures/browser-window-options.md) out of +this list (such as `width`, `height`, `x`, `y`, `show`, `frame`, `title`, +`backgroundColor`). Because the renderer is untrusted, options that cause the +main process to access the filesystem or that are otherwise privileged (such as +`icon`) are ignored. For full control and better ergonomics, use +`webContents.setWindowOpenHandler` to customize the BrowserWindow creation from +the main process. A subset of [`WebPreferences`](structures/web-preferences.md) can be set directly, unnested, from the features string: `zoomFactor`, `nodeIntegration`, `javascript`, @@ -56,9 +60,10 @@ window.open('https://github.com', '_blank', 'top=500,left=200,frame=false,nodeIn enabled on the parent window. * JavaScript will always be disabled in the opened `window` if it is disabled on the parent window. -* Non-standard features (that are not handled by Chromium or Electron) given in - `features` will be passed to any registered `webContents`'s - `did-create-window` event handler in the `options` argument. +* Features that are not handled by Chromium and not in Electron's allowlist of + presentational `BrowserWindowConstructorOptions` are ignored. The raw + `features` string is still available to the main process via + `setWindowOpenHandler`. * `frameName` follows the specification of `target` located in the [native documentation](https://developer.mozilla.org/en-US/docs/Web/API/Window/open#parameters). * When opening `about:blank`, the child window's [`WebPreferences`](structures/web-preferences.md) will be copied from the parent window, and there is no way to override it because Chromium diff --git a/lib/browser/parse-features-string.ts b/lib/browser/parse-features-string.ts index d14eb2f877..995b8e2a9b 100644 --- a/lib/browser/parse-features-string.ts +++ b/lib/browser/parse-features-string.ts @@ -78,6 +78,27 @@ export function parseWebViewWebPreferences (preferences: string) { const allowedWebPreferences = ['zoomFactor', 'nodeIntegration', 'javascript', 'contextIsolation', 'webviewTag'] as const; type AllowedWebPreference = (typeof allowedWebPreferences)[number]; +// Top-level BrowserWindow options that may be set via the window.open() +// features string. Options not listed here are silently dropped; apps that +// need to pass other options should use setWindowOpenHandler in the main +// process. +const allowedWindowOptions = new Set([ + // standard window.open() position/size features + 'top', 'left', 'innerWidth', 'innerHeight', + // numeric + 'x', 'y', 'width', 'height', + 'minWidth', 'minHeight', 'maxWidth', 'maxHeight', 'opacity', + // presentational booleans + 'show', 'center', 'useContentSize', 'frame', 'transparent', 'hasShadow', + 'movable', 'closable', 'focusable', 'minimizable', 'maximizable', + 'fullscreenable', 'alwaysOnTop', 'skipTaskbar', 'modal', 'acceptFirstMouse', + 'autoHideMenuBar', 'enableLargerThanScreen', 'paintWhenInitiallyHidden', + 'roundedCorners', 'thickFrame', 'disableAutoHideCursor', 'hiddenInMissionControl', + // presentational strings (no filesystem/network side effects) + 'title', 'backgroundColor', 'tabbingIdentifier', 'titleBarStyle', 'vibrancy', + 'visualEffectState', 'backgroundMaterial' +]); + /** * Parses a feature string that has the format used in window.open(). */ @@ -100,8 +121,15 @@ export function parseFeatures (features: string) { if (parsed.left !== undefined) parsed.x = parsed.left; if (parsed.top !== undefined) parsed.y = parsed.top; + const options: { [key: string]: CoercedValue } = {}; + for (const key of Object.keys(parsed)) { + if (allowedWindowOptions.has(key)) { + options[key] = parsed[key]; + } + } + return { - options: parsed as Omit, + options: options as Omit, webPreferences }; } diff --git a/spec/guest-window-manager-spec.ts b/spec/guest-window-manager-spec.ts index fbf4d44b2e..a1f27550fa 100644 --- a/spec/guest-window-manager-spec.ts +++ b/spec/guest-window-manager-spec.ts @@ -237,6 +237,32 @@ describe('webContents.setWindowOpenHandler', () => { expect(await browserWindow.webContents.executeJavaScript('42')).to.equal(42); }); + it('does not propagate non-allowlisted features-string options like icon to the BrowserWindow', async () => { + browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'allow' })); + + const didCreateWindow = once(browserWindow.webContents, 'did-create-window') as Promise<[BrowserWindow, Electron.DidCreateWindowDetails]>; + browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no,width=400,icon=/tmp/does-not-exist.png') && true"); + const [, details] = await didCreateWindow; + + expect(details.options).to.not.have.property('icon'); + expect(details.options.width).to.equal(400); + expect(details.options.show).to.equal(false); + }); + + it('still allows the main process to set icon via overrideBrowserWindowOptions', async () => { + const iconPath = nodePath.join(__dirname, 'fixtures', 'assets', 'icon.ico'); + browserWindow.webContents.setWindowOpenHandler(() => ({ + action: 'allow', + overrideBrowserWindowOptions: { icon: iconPath } + })); + + const didCreateWindow = once(browserWindow.webContents, 'did-create-window') as Promise<[BrowserWindow, Electron.DidCreateWindowDetails]>; + browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no,icon=/tmp/attacker.png') && true"); + const [, details] = await didCreateWindow; + + expect((details.options as any).icon).to.equal(iconPath); + }); + it('can open an offscreen child window from an onscreen parent', async () => { browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'allow', diff --git a/spec/parse-features-string-spec.ts b/spec/parse-features-string-spec.ts index 1392ae1582..02da849a5d 100644 --- a/spec/parse-features-string-spec.ts +++ b/spec/parse-features-string-spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { parseCommaSeparatedKeyValue } from '../lib/browser/parse-features-string'; +import { parseCommaSeparatedKeyValue, parseFeatures } from '../lib/browser/parse-features-string'; describe('feature-string parsing', () => { it('is indifferent to whitespace around keys and values', () => { @@ -19,4 +19,46 @@ describe('feature-string parsing', () => { checkParse(' a = yes , c = d', { a: true, c: 'd' }); checkParse(' a = yes , c = d ', { a: true, c: 'd' }); }); + + describe('parseFeatures allowlist', () => { + it('passes through allowlisted presentational options', () => { + const { options } = parseFeatures('width=400,height=300,show=no,frame=no,title=hi,backgroundColor=#fff,left=10,top=20'); + expect(options).to.deep.equal({ + width: 400, + height: 300, + show: false, + frame: false, + title: 'hi', + backgroundColor: '#fff', + left: 10, + top: 20, + x: 10, + y: 20 + }); + }); + + it('drops non-allowlisted options that would be unsafe to pass to BrowserWindow', () => { + const { options } = parseFeatures('icon=/etc/passwd,width=400'); + expect(options).to.deep.equal({ width: 400 }); + expect(options).to.not.have.property('icon'); + }); + + it('drops non-allowlisted options even when paired with UNC paths', () => { + const { options } = parseFeatures('icon=\\\\attacker.example\\share\\x.png,show=no'); + expect(options).to.deep.equal({ show: false }); + expect(options).to.not.have.property('icon'); + }); + + it('drops unknown options', () => { + const { options } = parseFeatures('something-unknown=foo,width=400'); + expect(options).to.deep.equal({ width: 400 }); + expect(options).to.not.have.property('something-unknown'); + }); + + it('still extracts allowlisted webPreferences', () => { + const { options, webPreferences } = parseFeatures('icon=/etc/passwd,nodeIntegration=no,width=400'); + expect(options).to.deep.equal({ width: 400 }); + expect(webPreferences).to.deep.equal({ nodeIntegration: false }); + }); + }); });