fix: a security issue related to open external (#13864)

This commit is contained in:
Peng Xiao
2025-11-06 20:54:25 +08:00
committed by GitHub
parent dd676611ce
commit c9a4129a3e
7 changed files with 133 additions and 13 deletions

View File

@@ -21,6 +21,7 @@ import type { HelperToMain, MainToHelper } from '../shared/type';
import { MessageEventChannel } from '../shared/utils'; import { MessageEventChannel } from '../shared/utils';
import { beforeAppQuit } from './cleanup'; import { beforeAppQuit } from './cleanup';
import { logger } from './logger'; import { logger } from './logger';
import { openExternalSafely } from './security/open-external';
const HELPER_PROCESS_PATH = path.join(__dirname, './helper.js'); const HELPER_PROCESS_PATH = path.join(__dirname, './helper.js');
@@ -105,10 +106,10 @@ class HelperProcessManager {
return dialog.showSaveDialog(window, opts); return dialog.showSaveDialog(window, opts);
}, },
}; };
const shellMethods = pickAndBind(shell, [ const shellMethods = {
'openExternal', openExternal: openExternalSafely as typeof shell.openExternal,
'showItemInFolder', showItemInFolder: shell.showItemInFolder.bind(shell),
]); };
const appMethods = pickAndBind(app, ['getPath']); const appMethods = pickAndBind(app, ['getPath']);
const mainToHelperServer: MainToHelper = { const mainToHelperServer: MainToHelper = {

View File

@@ -7,6 +7,7 @@ import path from 'node:path';
import { shell } from 'electron'; import { shell } from 'electron';
import { isMacOS } from '../../shared/utils'; import { isMacOS } from '../../shared/utils';
import { openExternalSafely } from '../security/open-external';
import type { NamespaceHandlers } from '../type'; import type { NamespaceHandlers } from '../type';
import { import {
askForMeetingPermission, askForMeetingPermission,
@@ -87,7 +88,9 @@ export const recordingHandlers = {
microphone: 'Privacy_Microphone', microphone: 'Privacy_Microphone',
}; };
const url = `x-apple.systempreferences:com.apple.preference.security?${urlMap[type]}`; const url = `x-apple.systempreferences:com.apple.preference.security?${urlMap[type]}`;
return shell.openExternal(url); return openExternalSafely(url, {
additionalProtocols: ['x-apple.systempreferences:'],
});
} }
// this only available on MacOS // this only available on MacOS
return false; return false;

View File

@@ -1,4 +1,35 @@
import { app, shell } from 'electron'; import { app } from 'electron';
import { openExternalSafely } from './security/open-external';
const extractRedirectTarget = (rawUrl: string) => {
try {
const parsed = new URL(rawUrl);
const redirectUri = parsed.searchParams.get('redirect_uri');
if (redirectUri) {
return redirectUri;
}
if (parsed.hash) {
const hash = parsed.hash.startsWith('#')
? parsed.hash.slice(1)
: parsed.hash;
const queryIndex = hash.indexOf('?');
if (queryIndex !== -1) {
const hashParams = new URLSearchParams(hash.slice(queryIndex + 1));
const hashRedirect = hashParams.get('redirect_uri');
if (hashRedirect) {
return hashRedirect;
}
}
}
return null;
} catch {
return null;
}
};
app.on('web-contents-created', (_, contents) => { app.on('web-contents-created', (_, contents) => {
const isInternalUrl = (url: string) => { const isInternalUrl = (url: string) => {
@@ -18,7 +49,9 @@ app.on('web-contents-created', (_, contents) => {
} }
// Prevent navigation // Prevent navigation
event.preventDefault(); event.preventDefault();
shell.openExternal(url).catch(console.error); openExternalSafely(url).catch(error => {
console.error('[security] Failed to open external URL:', error);
});
}); });
/** /**
@@ -32,9 +65,22 @@ app.on('web-contents-created', (_, contents) => {
* @see https://www.electronjs.org/docs/latest/tutorial/security#15-do-not-use-openexternal-with-untrusted-content * @see https://www.electronjs.org/docs/latest/tutorial/security#15-do-not-use-openexternal-with-untrusted-content
*/ */
contents.setWindowOpenHandler(({ url }) => { contents.setWindowOpenHandler(({ url }) => {
if (!isInternalUrl(url) || url.includes('/redirect-proxy')) { if (!isInternalUrl(url)) {
// Open default browser openExternalSafely(url).catch(error => {
shell.openExternal(url).catch(console.error); console.error('[security] Failed to open external URL:', error);
});
} else if (url.includes('/redirect-proxy')) {
const redirectTarget = extractRedirectTarget(url);
if (redirectTarget) {
openExternalSafely(redirectTarget).catch(error => {
console.error('[security] Failed to open external URL:', error);
});
} else {
console.warn(
'[security] Blocked redirect proxy with missing redirect target:',
url
);
}
} }
// Prevent creating new window in application // Prevent creating new window in application
return { action: 'deny' }; return { action: 'deny' };

View File

@@ -0,0 +1,52 @@
import { shell } from 'electron';
const DEFAULT_ALLOWED_PROTOCOLS = new Set(['http:', 'https:', 'mailto:']);
export interface OpenExternalOptions {
additionalProtocols?: string[];
}
export const isAllowedExternalUrl = (
rawUrl: string,
additionalProtocols: Iterable<string> = []
) => {
try {
const parsed = new URL(rawUrl);
const protocol = parsed.protocol.toLowerCase();
if (DEFAULT_ALLOWED_PROTOCOLS.has(protocol)) {
return true;
}
for (const extra of additionalProtocols) {
if (protocol === extra.toLowerCase()) {
return true;
}
}
return false;
} catch (error) {
console.warn('[security] Failed to parse external URL', rawUrl, error);
return false;
}
};
export const openExternalSafely = async (
rawUrl: string,
options: OpenExternalOptions = {}
) => {
const { additionalProtocols = [] } = options;
if (!isAllowedExternalUrl(rawUrl, additionalProtocols)) {
console.warn('[security] Blocked attempt to open external URL:', rawUrl);
return;
}
try {
await shell.openExternal(rawUrl);
} catch (error) {
console.error('[security] Failed to open external URL:', rawUrl, error);
}
};
export const ALLOWED_EXTERNAL_PROTOCOLS: ReadonlySet<string> = new Set(
DEFAULT_ALLOWED_PROTOCOLS
);

View File

@@ -1,9 +1,10 @@
import { app, clipboard, nativeImage, nativeTheme, shell } from 'electron'; import { app, clipboard, nativeImage, nativeTheme } from 'electron';
import { getLinkPreview } from 'link-preview-js'; import { getLinkPreview } from 'link-preview-js';
import { isMacOS } from '../../shared/utils'; import { isMacOS } from '../../shared/utils';
import { persistentConfig } from '../config-storage/persist'; import { persistentConfig } from '../config-storage/persist';
import { logger } from '../logger'; import { logger } from '../logger';
import { openExternalSafely } from '../security/open-external';
import type { WorkbenchViewMeta } from '../shared-state-schema'; import type { WorkbenchViewMeta } from '../shared-state-schema';
import type { NamespaceHandlers } from '../type'; import type { NamespaceHandlers } from '../type';
import { import {
@@ -151,7 +152,7 @@ export const uiHandlers = {
} }
}, },
openExternal(_, url: string) { openExternal(_, url: string) {
return shell.openExternal(url); return openExternalSafely(url);
}, },
// tab handlers // tab handlers

View File

@@ -14,6 +14,7 @@ const trustedDomain = [
]; ];
const logger = new DebugLogger('redirect_proxy'); const logger = new DebugLogger('redirect_proxy');
const ALLOWED_PROTOCOLS = new Set(['http:', 'https:']);
/** /**
* /redirect-proxy page * /redirect-proxy page
@@ -32,6 +33,11 @@ export const loader: LoaderFunction = async ({ request }) => {
try { try {
const target = new URL(redirectUri); const target = new URL(redirectUri);
if (!ALLOWED_PROTOCOLS.has(target.protocol)) {
logger.warn('Blocked redirect with disallowed protocol', target.protocol);
return { allow: false };
}
if ( if (
target.hostname === window.location.hostname || target.hostname === window.location.hostname ||
trustedDomain.some(domain => trustedDomain.some(domain =>
@@ -46,7 +52,8 @@ export const loader: LoaderFunction = async ({ request }) => {
return { allow: false }; return { allow: false };
} }
return { allow: true }; logger.warn('Blocked redirect to untrusted domain', redirectUri);
return { allow: false };
}; };
export const Component = () => { export const Component = () => {

View File

@@ -39,6 +39,16 @@ export class UrlService extends Service {
* @param url only full url with http/https protocol is supported * @param url only full url with http/https protocol is supported
*/ */
openExternal(url: string) { openExternal(url: string) {
let parsed: URL;
try {
parsed = new URL(url);
} catch {
throw new Error(`Invalid external URL: ${url}`);
}
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
throw new Error('only http/https URLs are supported');
}
if (BUILD_CONFIG.isWeb || BUILD_CONFIG.isMobileWeb) { if (BUILD_CONFIG.isWeb || BUILD_CONFIG.isMobileWeb) {
location.href = url; location.href = url;
} else { } else {