diff --git a/packages/cli-server-api/package.json b/packages/cli-server-api/package.json index 37355cbb2..d020ea1af 100644 --- a/packages/cli-server-api/package.json +++ b/packages/cli-server-api/package.json @@ -16,7 +16,6 @@ "open": "^6.2.0", "pretty-format": "^29.7.0", "serve-static": "^1.13.1", - "strict-url-sanitise": "0.0.1", "ws": "^6.2.3" }, "devDependencies": { diff --git a/packages/cli-server-api/src/__tests__/openURLMiddleware.test.ts b/packages/cli-server-api/src/__tests__/openURLMiddleware.test.ts index 0f26e8274..12c0f984d 100644 --- a/packages/cli-server-api/src/__tests__/openURLMiddleware.test.ts +++ b/packages/cli-server-api/src/__tests__/openURLMiddleware.test.ts @@ -5,8 +5,11 @@ import openURLMiddleware from '../openURLMiddleware'; jest.mock('open'); -function createMockRequest(method: string, body: object): http.IncomingMessage { - const bodyStr = JSON.stringify(body); +function createMockRequest( + method: string, + body?: object, +): http.IncomingMessage { + const bodyStr = body == null ? '' : JSON.stringify(body); const readable = new Readable(); readable.push(bodyStr); readable.push(null); @@ -21,18 +24,45 @@ function createMockRequest(method: string, body: object): http.IncomingMessage { }) as unknown as http.IncomingMessage; } -describe('openURLMiddleware', () => { - let res: jest.Mocked; - let next: jest.Mock; - - beforeEach(() => { - res = { - writeHead: jest.fn(), - end: jest.fn(), +type MiddlewareResponse = { + body?: string; + next: jest.Mock; + statusCode?: number; +}; + +function callOpenURLMiddleware( + body?: object, + method = 'POST', +): Promise { + return new Promise((resolve, reject) => { + const response: MiddlewareResponse = { + next: jest.fn((error?: Error) => { + if (error) { + reject(error); + return; + } + + resolve(response); + }), + }; + + const res = { + writeHead: jest.fn((statusCode: number) => { + response.statusCode = statusCode; + }), + end: jest.fn((message?: string) => { + response.body = message; + resolve(response); + }), setHeader: jest.fn(), } as any; - next = jest.fn(); + openURLMiddleware(createMockRequest(method, body), res, response.next); + }); +} + +describe('openURLMiddleware', () => { + beforeEach(() => { jest.clearAllMocks(); }); @@ -40,78 +70,57 @@ describe('openURLMiddleware', () => { jest.restoreAllMocks(); }); - test('should return 400 for non-string URL', (done) => { - const req = createMockRequest('POST', {url: 123}); - - res.end = jest.fn(() => { - try { - expect(open).not.toHaveBeenCalled(); - expect(res.writeHead).toHaveBeenCalledWith(400); - expect(res.end).toHaveBeenCalledWith('URL must be a string'); - done(); - } catch (error) { - done(error); - } - }) as any; - - openURLMiddleware(req, res, next); - }); + test.each([ + 'https://reactnative.dev/docs/tutorial', + 'https://reactnative.dev/docs/fast-refresh', + 'https://x.com/reactnative', + ])('should open React Native welcome screen URL %s', async (url) => { + const response = await callOpenURLMiddleware({url}); - // CVE-2025-11953 - test('should reject malicious URL with invalid hostname', (done) => { - const maliciousUrl = 'https://www.$(calc.exe).com/foo'; - const req = createMockRequest('POST', {url: maliciousUrl}); - - res.end = jest.fn(() => { - try { - expect(open).not.toHaveBeenCalled(); - expect(res.writeHead).toHaveBeenCalledWith(400); - expect(res.end).toHaveBeenCalledWith('Invalid URL'); - done(); - } catch (error) { - done(error); - } - }) as any; - - openURLMiddleware(req, res, next); + expect(open).toHaveBeenCalledWith(url); + expect(response.statusCode).toBe(200); + expect(response.next).not.toHaveBeenCalled(); }); - // CVE-2025-11953 - test('should reject URL with Windows pipe separator', (done) => { - const maliciousUrl = 'https://evil.com?|calc.exe'; - const req = createMockRequest('POST', {url: maliciousUrl}); - - res.end = jest.fn(() => { - try { - expect(open).not.toHaveBeenCalled(); - expect(res.writeHead).toHaveBeenCalledWith(400); - expect(res.end).toHaveBeenCalledWith('Invalid URL'); - done(); - } catch (error) { - done(error); - } - }) as any; - - openURLMiddleware(req, res, next); + test('should return 400 for non-string URL', async () => { + const response = await callOpenURLMiddleware({url: 123}); + + expect(open).not.toHaveBeenCalled(); + expect(response.statusCode).toBe(400); + expect(response.body).toBe('URL must be a string'); }); // CVE-2025-11953 - test('should reject URL with Windows command exfiltration', (done) => { - // Encodes to reveal %BETA% env var - const maliciousUrl = 'https://example.com/?a=%¾TA%'; - const req = createMockRequest('POST', {url: maliciousUrl}); - - res.end = jest.fn(() => { - try { - expect(open).not.toHaveBeenCalled(); - expect(res.writeHead).toHaveBeenCalledWith(400); - expect(res.end).toHaveBeenCalledWith('Invalid URL'); - done(); - } catch (error) { - done(error); - } - }) as any; - - openURLMiddleware(req, res, next); + test.each([ + ['JFrog bare executable command', 'calc.exe'], + ['JFrog nested cmd RCE command', 'cmd /c echo abc > c:\\temp\\pwned.txt'], + ['Windows command prefix', '& calc.exe'], + [ + 'URL followed by Windows command separator', + 'https://example.com & calc.exe', + ], + ['malicious URL with invalid hostname', 'https://www.$(calc.exe).com/foo'], + ['URL with Windows pipe separator', 'https://evil.com?|calc.exe'], + ['URL with Windows caret separator', 'https://example.com/?x=^calc'], + ['URL with Windows command exfiltration', 'https://example.com/?a=%¾TA%'], + ['URL with Windows delayed expansion', 'https://example.com/?x=!PATH!'], + [ + 'URL with Windows redirect metacharacter', + 'https://example.com/?x=>out.txt', + ], + [ + 'URL with Windows metacharacter in userinfo', + 'https://u:p|ss@example.com/', + ], + ['file URL scheme', 'file:///etc/passwd'], + ['javascript URL scheme', 'javascript:alert(1)'], + ['custom URL scheme', 'ms-msdt:/id'], + ['IPv6 hostname with injected metacharacter', 'https://[::1|x]/'], + ])('should reject %s', async (_name, url) => { + const response = await callOpenURLMiddleware({url}); + + expect(open).not.toHaveBeenCalled(); + expect(response.statusCode).toBe(400); + expect(response.body).toBe('Invalid URL'); }); }); diff --git a/packages/cli-server-api/src/openURLMiddleware.ts b/packages/cli-server-api/src/openURLMiddleware.ts index 14d5eae1a..a8df894c2 100644 --- a/packages/cli-server-api/src/openURLMiddleware.ts +++ b/packages/cli-server-api/src/openURLMiddleware.ts @@ -10,7 +10,46 @@ import type {IncomingMessage, ServerResponse} from 'http'; import {json} from 'body-parser'; import connect from 'connect'; import open from 'open'; -import {sanitizeUrl} from 'strict-url-sanitise'; + +// open@6 launches URLs through `cmd /c start` on Windows and only escapes `&`. +// Reject the remaining cmd metacharacters, including `%` and `!` expansion, +// even though this also rejects some otherwise-valid percent-encoded URLs. +const WINDOWS_SHELL_SPECIAL_CHARS = /[|<>^%!]/; +const INVALID_URL = 'Invalid URL'; + +function sendResponse( + res: ServerResponse, + statusCode: number, + message?: string, +) { + res.writeHead(statusCode); + res.end(message); +} + +function isSafeHostname(hostname: string) { + return ( + (hostname.startsWith('[') && hostname.endsWith(']')) || + hostname === encodeURIComponent(hostname) + ); +} + +function validateURLForOpen(url: string) { + const parsedUrl = new URL(url); + + if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') { + throw new Error('Invalid URL protocol'); + } + + if (!isSafeHostname(parsedUrl.hostname)) { + throw new Error('Invalid URL hostname'); + } + + if (WINDOWS_SHELL_SPECIAL_CHARS.test(parsedUrl.href)) { + throw new Error('Invalid URL characters'); + } + + return parsedUrl.href; +} /** * Open a URL in the system browser. @@ -25,32 +64,29 @@ async function openURLMiddleware( ) { if (req.method === 'POST') { if (req.body == null) { - res.writeHead(400); - res.end('Missing request body'); + sendResponse(res, 400, 'Missing request body'); return; } const {url} = req.body as {url: string}; if (typeof url !== 'string') { - res.writeHead(400); - res.end('URL must be a string'); + sendResponse(res, 400, 'URL must be a string'); return; } - let sanitizedUrl: string; + let validatedUrl; try { - sanitizedUrl = sanitizeUrl(url); + validatedUrl = validateURLForOpen(url); } catch { - res.writeHead(400); - res.end('Invalid URL'); + sendResponse(res, 400, INVALID_URL); return; } - await open(sanitizedUrl); + await open(validatedUrl); - res.writeHead(200); - res.end(); + sendResponse(res, 200); + return; } next(); diff --git a/yarn.lock b/yarn.lock index 055d4ab9a..0a66d3cbf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9922,11 +9922,6 @@ stream-buffers@2.2.x: resolved "https://registry.yarnpkg.com/stream-buffers/-/stream-buffers-2.2.0.tgz#91d5f5130d1cef96dcfa7f726945188741d09ee4" integrity sha512-uyQK/mx5QjHun80FLJTfaWE7JtwfRMKBLkMne6udYOmvH0CawotVa7TfgYHzAnpphn4+TweIx1QKMnRIbipmUg== -strict-url-sanitise@0.0.1: - version "0.0.1" - resolved "https://registry.yarnpkg.com/strict-url-sanitise/-/strict-url-sanitise-0.0.1.tgz#10cfac63c9dfdd856d98ab9f76433dad5ce99e0c" - integrity sha512-nuFtF539K8jZg3FjaWH/L8eocCR6gegz5RDOsaWxfdbF5Jqr2VXWxZayjTwUzsWJDC91k2EbnJXp6FuWW+Z4hg== - string-argv@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/string-argv/-/string-argv-0.3.1.tgz#95e2fbec0427ae19184935f816d74aaa4c5c19da"