fix: restrict window.open features to allowlisted BrowserWindow options (#50949)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>

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

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<string>([
+  // 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<BrowserWindowConstructorOptions, 'webPreferences'>,
+    options: options as Omit<BrowserWindowConstructorOptions, 'webPreferences'>,
     webPreferences
   };
 }

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',

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