diff --git a/goldens/public-api/angular/ssr/index.api.md b/goldens/public-api/angular/ssr/index.api.md index e5d85138b72f..a9d1d1b90176 100644 --- a/goldens/public-api/angular/ssr/index.api.md +++ b/goldens/public-api/angular/ssr/index.api.md @@ -22,6 +22,7 @@ export class AngularAppEngine { // @public export interface AngularAppEngineOptions { allowedHosts?: readonly string[]; + allowedProxyHeaders?: boolean | readonly string[]; } // @public diff --git a/goldens/public-api/angular/ssr/node/index.api.md b/goldens/public-api/angular/ssr/node/index.api.md index 2c52c06b47c1..52d0b31bd6e6 100644 --- a/goldens/public-api/angular/ssr/node/index.api.md +++ b/goldens/public-api/angular/ssr/node/index.api.md @@ -55,7 +55,7 @@ export interface CommonEngineRenderOptions { export function createNodeRequestHandler(handler: T): T; // @public -export function createWebRequestFromNodeRequest(nodeRequest: IncomingMessage | Http2ServerRequest): Request; +export function createWebRequestFromNodeRequest(nodeRequest: IncomingMessage | Http2ServerRequest, allowedProxyHeaders?: boolean | readonly string[]): Request; // @public export function isMainModule(url: string): boolean; diff --git a/packages/angular/ssr/node/src/app-engine.ts b/packages/angular/ssr/node/src/app-engine.ts index 7f7d3017a846..1945ed4f66a6 100644 --- a/packages/angular/ssr/node/src/app-engine.ts +++ b/packages/angular/ssr/node/src/app-engine.ts @@ -29,6 +29,7 @@ export interface AngularNodeAppEngineOptions extends AngularAppEngineOptions {} */ export class AngularNodeAppEngine { private readonly angularAppEngine: AngularAppEngine; + private readonly allowedProxyHeaders?: boolean | readonly string[]; /** * Creates a new instance of the Angular Node.js server application engine. @@ -39,6 +40,7 @@ export class AngularNodeAppEngine { ...options, allowedHosts: [...getAllowedHostsFromEnv(), ...(options?.allowedHosts ?? [])], }); + this.allowedProxyHeaders = options?.allowedProxyHeaders; attachNodeGlobalErrorHandlers(); } @@ -75,7 +77,9 @@ export class AngularNodeAppEngine { requestContext?: unknown, ): Promise { const webRequest = - request instanceof Request ? request : createWebRequestFromNodeRequest(request); + request instanceof Request + ? request + : createWebRequestFromNodeRequest(request, this.allowedProxyHeaders); return this.angularAppEngine.handle(webRequest, requestContext); } diff --git a/packages/angular/ssr/node/src/request.ts b/packages/angular/ssr/node/src/request.ts index 402ec29ba56d..06cefbdaabb5 100644 --- a/packages/angular/ssr/node/src/request.ts +++ b/packages/angular/ssr/node/src/request.ts @@ -17,7 +17,13 @@ import { getFirstHeaderValue } from '../../src/utils/validation'; * as they are not allowed to be set directly using the `Node.js` Undici API or * the web `Headers` API. */ -const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path', ':status']); +const HTTP2_PSEUDO_HEADERS: ReadonlySet = new Set([ + ':method', + ':scheme', + ':authority', + ':path', + ':status', +]); /** * Converts a Node.js `IncomingMessage` or `Http2ServerRequest` into a @@ -27,18 +33,31 @@ const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path * be used by web platform APIs. * * @param nodeRequest - The Node.js request object (`IncomingMessage` or `Http2ServerRequest`) to convert. + * @param allowedProxyHeaders - A boolean or an array of allowed proxy headers. + * + * @remarks + * When `allowedProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and + * `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure + * level (e.g., at the reverse proxy or API gateway) before reaching the application. + * * @returns A Web Standard `Request` object. */ export function createWebRequestFromNodeRequest( nodeRequest: IncomingMessage | Http2ServerRequest, + allowedProxyHeaders?: boolean | readonly string[], ): Request { + const allowedProxyHeadersNormalized = + allowedProxyHeaders && typeof allowedProxyHeaders !== 'boolean' + ? new Set(allowedProxyHeaders.map((h) => h.toLowerCase())) + : allowedProxyHeaders; + const { headers, method = 'GET' } = nodeRequest; const withBody = method !== 'GET' && method !== 'HEAD'; const referrer = headers.referer && URL.canParse(headers.referer) ? headers.referer : undefined; - return new Request(createRequestUrl(nodeRequest), { + return new Request(createRequestUrl(nodeRequest, allowedProxyHeadersNormalized), { method, - headers: createRequestHeaders(headers), + headers: createRequestHeaders(headers, allowedProxyHeadersNormalized), body: withBody ? nodeRequest : undefined, duplex: withBody ? 'half' : undefined, referrer, @@ -49,9 +68,13 @@ export function createWebRequestFromNodeRequest( * Creates a `Headers` object from Node.js `IncomingHttpHeaders`. * * @param nodeHeaders - The Node.js `IncomingHttpHeaders` object to convert. + * @param allowedProxyHeaders - A boolean or a set of allowed proxy headers. * @returns A `Headers` object containing the converted headers. */ -function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers { +function createRequestHeaders( + nodeHeaders: IncomingHttpHeaders, + allowedProxyHeaders: boolean | ReadonlySet | undefined, +): Headers { const headers = new Headers(); for (const [name, value] of Object.entries(nodeHeaders)) { @@ -59,6 +82,10 @@ function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers { continue; } + if (name.startsWith('x-forwarded-') && !isProxyHeaderAllowed(name, allowedProxyHeaders)) { + continue; + } + if (typeof value === 'string') { headers.append(name, value); } else if (Array.isArray(value)) { @@ -75,20 +102,34 @@ function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers { * Creates a `URL` object from a Node.js `IncomingMessage`, taking into account the protocol, host, and port. * * @param nodeRequest - The Node.js `IncomingMessage` or `Http2ServerRequest` object to extract URL information from. + * @param allowedProxyHeaders - A boolean or a set of allowed proxy headers. + * + * @remarks + * When `allowedProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and + * `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure + * level (e.g., at the reverse proxy or API gateway) before reaching the application. + * * @returns A `URL` object representing the request URL. */ -export function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerRequest): URL { +export function createRequestUrl( + nodeRequest: IncomingMessage | Http2ServerRequest, + allowedProxyHeaders?: boolean | ReadonlySet, +): URL { const { headers, socket, url = '', originalUrl, } = nodeRequest as IncomingMessage & { originalUrl?: string }; + const protocol = - getFirstHeaderValue(headers['x-forwarded-proto']) ?? + getAllowedProxyHeaderValue(headers, 'x-forwarded-proto', allowedProxyHeaders) ?? ('encrypted' in socket && socket.encrypted ? 'https' : 'http'); + const hostname = - getFirstHeaderValue(headers['x-forwarded-host']) ?? headers.host ?? headers[':authority']; + getAllowedProxyHeaderValue(headers, 'x-forwarded-host', allowedProxyHeaders) ?? + headers.host ?? + headers[':authority']; if (Array.isArray(hostname)) { throw new Error('host value cannot be an array.'); @@ -96,7 +137,7 @@ export function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerReque let hostnameWithPort = hostname; if (!hostname?.includes(':')) { - const port = getFirstHeaderValue(headers['x-forwarded-port']); + const port = getAllowedProxyHeaderValue(headers, 'x-forwarded-port', allowedProxyHeaders); if (port) { hostnameWithPort += `:${port}`; } @@ -104,3 +145,43 @@ export function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerReque return new URL(`${protocol}://${hostnameWithPort}${originalUrl ?? url}`); } + +/** + * Gets the first value of an allowed proxy header. + * + * @param headers - The Node.js incoming HTTP headers. + * @param headerName - The name of the proxy header to retrieve. + * @param allowedProxyHeaders - A boolean or a set of allowed proxy headers. + * @returns The value of the allowed proxy header, or `undefined` if not allowed or not present. + */ +function getAllowedProxyHeaderValue( + headers: IncomingHttpHeaders, + headerName: string, + allowedProxyHeaders: boolean | ReadonlySet | undefined, +): string | undefined { + return isProxyHeaderAllowed(headerName, allowedProxyHeaders) + ? getFirstHeaderValue(headers[headerName]) + : undefined; +} + +/** + * Checks if a specific proxy header is allowed. + * + * @param headerName - The name of the proxy header to check. + * @param allowedProxyHeaders - A boolean or a set of allowed proxy headers. + * @returns `true` if the header is allowed, `false` otherwise. + */ +function isProxyHeaderAllowed( + headerName: string, + allowedProxyHeaders: boolean | ReadonlySet | undefined, +): boolean { + if (!allowedProxyHeaders) { + return false; + } + + if (allowedProxyHeaders === true) { + return true; + } + + return allowedProxyHeaders.has(headerName.toLowerCase()); +} diff --git a/packages/angular/ssr/node/test/request_spec.ts b/packages/angular/ssr/node/test/request_spec.ts index 952faefd9f28..5d6168d3ecc1 100644 --- a/packages/angular/ssr/node/test/request_spec.ts +++ b/packages/angular/ssr/node/test/request_spec.ts @@ -137,6 +137,7 @@ describe('createRequestUrl', () => { }, url: '/test', }), + true, ); expect(url.href).toBe('https://example.com/test'); }); @@ -152,6 +153,7 @@ describe('createRequestUrl', () => { }, url: '/test', }), + true, ); expect(url.href).toBe('https://example.com:8443/test'); }); diff --git a/packages/angular/ssr/src/app-engine.ts b/packages/angular/ssr/src/app-engine.ts index 09e1093fef72..74b9c20da64d 100644 --- a/packages/angular/ssr/src/app-engine.ts +++ b/packages/angular/ssr/src/app-engine.ts @@ -12,7 +12,7 @@ import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n'; import { EntryPointExports, getAngularAppEngineManifest } from './manifest'; import { createRedirectResponse } from './utils/redirect'; import { joinUrlParts } from './utils/url'; -import { cloneRequestAndPatchHeaders, validateRequest } from './utils/validation'; +import { sanitizeRequestHeaders, validateRequest } from './utils/validation'; /** * Options for the Angular server application engine. @@ -22,6 +22,22 @@ export interface AngularAppEngineOptions { * A set of allowed hostnames for the server application. */ allowedHosts?: readonly string[]; + + /** + * Extends the scope of trusted proxy headers (`X-Forwarded-*`). + * + * @remarks + * When `allowedProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and + * `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure + * level (e.g., at the reverse proxy or API gateway) before reaching the application. + * + * If a `string[]` is provided, only those proxy headers are allowed. + * If `true`, all proxy headers are allowed. + * If `false` or not provided, proxy headers are ignored. + * + * @default false + */ + allowedProxyHeaders?: boolean | readonly string[]; } /** @@ -78,6 +94,11 @@ export class AngularAppEngine { this.manifest.supportedLocales, ); + /** + * The normalized allowed proxy headers. + */ + private readonly allowedProxyHeaders: ReadonlySet | boolean; + /** * A cache that holds entry points, keyed by their potential locale string. */ @@ -89,6 +110,12 @@ export class AngularAppEngine { */ constructor(options?: AngularAppEngineOptions) { this.allowedHosts = this.getAllowedHosts(options); + + const allowedProxyHeaders = options?.allowedProxyHeaders ?? false; + this.allowedProxyHeaders = + typeof allowedProxyHeaders === 'boolean' + ? allowedProxyHeaders + : new Set(allowedProxyHeaders.map((h) => h.toLowerCase())); } private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet { @@ -131,33 +158,17 @@ export class AngularAppEngine { */ async handle(request: Request, requestContext?: unknown): Promise { const allowedHost = this.allowedHosts; - const disableAllowedHostsCheck = AngularAppEngine.ɵdisableAllowedHostsCheck; + const securedRequest = sanitizeRequestHeaders(request, this.allowedProxyHeaders); try { - validateRequest(request, allowedHost, disableAllowedHostsCheck); + validateRequest(securedRequest, allowedHost, AngularAppEngine.ɵdisableAllowedHostsCheck); } catch (error) { - return this.handleValidationError(request.url, error as Error); + return this.handleValidationError(securedRequest.url, error as Error); } - // Clone request with patched headers to prevent unallowed host header access. - const { request: securedRequest, onError: onHeaderValidationError } = disableAllowedHostsCheck - ? { request, onError: null } - : cloneRequestAndPatchHeaders(request, allowedHost); - const serverApp = await this.getAngularServerAppForRequest(securedRequest); if (serverApp) { - const promises: Promise[] = []; - if (onHeaderValidationError) { - promises.push( - onHeaderValidationError.then((error) => - this.handleValidationError(securedRequest.url, error), - ), - ); - } - - promises.push(serverApp.handle(securedRequest, requestContext)); - - return Promise.race(promises); + return serverApp.handle(securedRequest, requestContext); } if (this.supportedLocales.length > 1) { diff --git a/packages/angular/ssr/src/utils/validation.ts b/packages/angular/ssr/src/utils/validation.ts index e8af64ed9943..75b60ccf8b53 100644 --- a/packages/angular/ssr/src/utils/validation.ts +++ b/packages/angular/ssr/src/utils/validation.ts @@ -9,7 +9,7 @@ /** * The set of headers that should be validated for host header injection attacks. */ -const HOST_HEADERS_TO_VALIDATE: ReadonlySet = new Set(['host', 'x-forwarded-host']); +const HOST_HEADERS_TO_VALIDATE: ReadonlyArray = ['host', 'x-forwarded-host']; /** * Regular expression to validate that the port is a numeric value. @@ -21,15 +21,10 @@ const VALID_PORT_REGEX = /^\d+$/; */ const VALID_PROTO_REGEX = /^https?$/i; -/** - * Regular expression to validate that the host is a valid hostname. - */ -const VALID_HOST_REGEX = /^[a-z0-9_.-]+(:[0-9]+)?$/i; - /** * Regular expression to validate that the prefix is valid. */ -const INVALID_PREFIX_REGEX = /^(?:\\|\/[/\\])|(?:^|[/\\])\.\.?(?:[/\\]|$)/; +const VALID_PREFIX_REGEX = /^\/([a-z0-9_-]+\/)*[a-z0-9_-]*$/i; /** * Extracts the first value from a multi-value header string. @@ -64,7 +59,7 @@ export function validateRequest( allowedHosts: ReadonlySet, disableHostCheck: boolean, ): void { - validateHeaders(request); + validateHeaders(request, allowedHosts, disableHostCheck); if (!disableHostCheck) { validateUrl(new URL(request.url), allowedHosts); @@ -86,119 +81,46 @@ export function validateUrl(url: URL, allowedHosts: ReadonlySet): void { } /** - * Clones a request and patches the `get` method of the request headers to validate the host headers. - * @param request - The request to validate. - * @param allowedHosts - A set of allowed hostnames. - * @returns An object containing the cloned request and a promise that resolves to an error - * if any of the validated headers contain invalid values. + * Sanitizes the proxy headers of a request by removing unallowed `X-Forwarded-*` headers. + * If no headers need to be removed, the original request is returned without cloning. + * + * @param request - The incoming `Request` object to sanitize. + * @param allowedProxyHeaders - A set of allowed proxy headers or a boolean indicating whether all are allowed. + * @returns The sanitized request, or the original request if no changes were needed. */ -export function cloneRequestAndPatchHeaders( +export function sanitizeRequestHeaders( request: Request, - allowedHosts: ReadonlySet, -): { request: Request; onError: Promise } { - let onError: (value: Error) => void; - const onErrorPromise = new Promise((resolve) => { - onError = resolve; - }); - - const clonedReq = new Request(request.clone(), { - signal: request.signal, - }); - - const headers = clonedReq.headers; - - const originalGet = headers.get; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - (headers.get as typeof originalGet) = function (name) { - const value = originalGet.call(headers, name); - if (!value) { - return value; - } - - validateHeader(name, value, allowedHosts, onError); - - return value; - }; + allowedProxyHeaders?: boolean | ReadonlySet, +): Request { + if (allowedProxyHeaders === true) { + return request; + } - const originalValues = headers.values; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - (headers.values as typeof originalValues) = function () { - for (const name of HOST_HEADERS_TO_VALIDATE) { - validateHeader(name, originalGet.call(headers, name), allowedHosts, onError); + const keysToDelete: string[] = []; + for (const [key] of request.headers) { + const lowerKey = key.toLowerCase(); + if ( + lowerKey.startsWith('x-forwarded-') && + (!allowedProxyHeaders || !allowedProxyHeaders.has(lowerKey)) + ) { + keysToDelete.push(key); } - - return originalValues.call(headers); - }; - - const originalEntries = headers.entries; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - (headers.entries as typeof originalEntries) = function () { - const iterator = originalEntries.call(headers); - - return { - next() { - const result = iterator.next(); - if (!result.done) { - const [key, value] = result.value; - validateHeader(key, value, allowedHosts, onError); - } - - return result; - }, - [Symbol.iterator]() { - return this; - }, - }; - }; - - const originalForEach = headers.forEach; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - (headers.forEach as typeof originalForEach) = function (callback, thisArg) { - originalForEach.call( - headers, - (value, key, parent) => { - validateHeader(key, value, allowedHosts, onError); - callback.call(thisArg, value, key, parent); - }, - thisArg, - ); - }; - - // Ensure for...of loops use the new patched entries - (headers[Symbol.iterator] as typeof originalEntries) = headers.entries; - - return { request: clonedReq, onError: onErrorPromise }; -} - -/** - * Validates a specific header value against the allowed hosts. - * @param name - The name of the header to validate. - * @param value - The value of the header to validate. - * @param allowedHosts - A set of allowed hostnames. - * @param onError - A callback function to call if the header value is invalid. - * @throws Error if the header value is invalid. - */ -function validateHeader( - name: string, - value: string | null, - allowedHosts: ReadonlySet, - onError: (value: Error) => void, -): void { - if (!value) { - return; } - if (!HOST_HEADERS_TO_VALIDATE.has(name.toLowerCase())) { - return; + if (keysToDelete.length === 0) { + return request; } - try { - verifyHostAllowed(name, value, allowedHosts); - } catch (error) { - onError(error as Error); + const clonedReq = new Request(request.clone(), { + signal: request.signal, + }); - throw error; + const headers = clonedReq.headers; + for (const key of keysToDelete) { + headers.delete(key); } + + return clonedReq; } /** @@ -214,19 +136,20 @@ function verifyHostAllowed( headerValue: string, allowedHosts: ReadonlySet, ): void { - const value = getFirstHeaderValue(headerValue); - if (!value) { - return; - } - - const url = `http://${value}`; + const url = `http://${headerValue}`; if (!URL.canParse(url)) { throw new Error(`Header "${headerName}" contains an invalid value and cannot be parsed.`); } - const { hostname } = new URL(url); + const { hostname, pathname, search, hash, username, password } = new URL(url); + if (pathname !== '/' || search || hash || username || password) { + throw new Error( + `Header "${headerName}" with value "${headerValue}" contains characters that are not allowed.`, + ); + } + if (!isHostAllowed(hostname, allowedHosts)) { - throw new Error(`Header "${headerName}" with value "${value}" is not allowed.`); + throw new Error(`Header "${headerName}" with value "${headerValue}" is not allowed.`); } } @@ -259,14 +182,20 @@ function isHostAllowed(hostname: string, allowedHosts: ReadonlySet): boo * Validates the headers of an incoming request. * * @param request - The incoming `Request` object containing the headers to validate. + * @param allowedHosts - A set of allowed hostnames. + * @param disableHostCheck - Whether to disable the host check. * @throws Error if any of the validated headers contain invalid values. */ -function validateHeaders(request: Request): void { +function validateHeaders( + request: Request, + allowedHosts: ReadonlySet, + disableHostCheck: boolean, +): void { const headers = request.headers; for (const headerName of HOST_HEADERS_TO_VALIDATE) { const headerValue = getFirstHeaderValue(headers.get(headerName)); - if (headerValue && !VALID_HOST_REGEX.test(headerValue)) { - throw new Error(`Header "${headerName}" contains characters that are not allowed.`); + if (headerValue && !disableHostCheck) { + verifyHostAllowed(headerName, headerValue, allowedHosts); } } @@ -281,9 +210,10 @@ function validateHeaders(request: Request): void { } const xForwardedPrefix = getFirstHeaderValue(headers.get('x-forwarded-prefix')); - if (xForwardedPrefix && INVALID_PREFIX_REGEX.test(xForwardedPrefix)) { + if (xForwardedPrefix && !VALID_PREFIX_REGEX.test(xForwardedPrefix)) { throw new Error( - 'Header "x-forwarded-prefix" must not start with "\\" or multiple "/" or contain ".", ".." path segments.', + 'Header "x-forwarded-prefix" is invalid. It must start with a "/" and contain ' + + 'only alphanumeric characters, hyphens, and underscores, separated by single slashes.', ); } } diff --git a/packages/angular/ssr/test/app-engine_spec.ts b/packages/angular/ssr/test/app-engine_spec.ts index 14e664659748..4deff74d8de3 100644 --- a/packages/angular/ssr/test/app-engine_spec.ts +++ b/packages/angular/ssr/test/app-engine_spec.ts @@ -105,7 +105,7 @@ describe('AngularAppEngine', () => { basePath: '/', }); - appEngine = new AngularAppEngine(); + appEngine = new AngularAppEngine({ allowedProxyHeaders: true }); }); describe('handle', () => { @@ -177,6 +177,21 @@ describe('AngularAppEngine', () => { expect(response?.headers.get('Vary')).toBe('X-Forwarded-Prefix, Accept-Language'); }); + it('should completely ignore proxy headers if not allowed', async () => { + const strictEngine = new AngularAppEngine({ allowedProxyHeaders: false }); + const request = new Request('https://example.com', { + headers: { + 'Accept-Language': 'it', + 'X-Forwarded-Prefix': '/app', + }, + }); + + // The strictEngine will ignore the prefix + const response = await strictEngine.handle(request); + expect(response?.status).toBe(302); + expect(response?.headers.get('Location')).toBe('/it'); + }); + it('should return null for requests to file-like resources in a locale', async () => { const request = new Request('https://example.com/it/logo.png'); const response = await appEngine.handle(request); @@ -324,7 +339,7 @@ describe('AngularAppEngine', () => { supportedLocales: { 'en-US': '' }, }); - appEngine = new AngularAppEngine(); + appEngine = new AngularAppEngine({ allowedProxyHeaders: true }); }); beforeEach(() => { @@ -380,10 +395,12 @@ describe('AngularAppEngine', () => { expect(response).not.toBeNull(); expect(response?.status).toBe(400); expect(await response?.text()).toContain( - 'Header "host" contains characters that are not allowed.', + 'Header "host" with value "example.com/evil" contains characters that are not allowed.', ); expect(consoleErrorSpy).toHaveBeenCalledWith( - jasmine.stringMatching('Header "host" contains characters that are not allowed.'), + jasmine.stringMatching( + 'Header "host" with value "example.com/evil" contains characters that are not allowed.', + ), ); }); }); diff --git a/packages/angular/ssr/test/utils/validation_spec.ts b/packages/angular/ssr/test/utils/validation_spec.ts index 5c4b6e8cd121..b0004b06ad8a 100644 --- a/packages/angular/ssr/test/utils/validation_spec.ts +++ b/packages/angular/ssr/test/utils/validation_spec.ts @@ -7,8 +7,8 @@ */ import { - cloneRequestAndPatchHeaders, getFirstHeaderValue, + sanitizeRequestHeaders, validateRequest, validateUrl, } from '../../src/utils/validation'; @@ -132,7 +132,7 @@ describe('Validation Utils', () => { headers: { 'host': 'example.com/bad' }, }); expect(() => validateRequest(req, allowedHosts, false)).toThrowError( - 'Header "host" contains characters that are not allowed.', + 'Header "host" with value "example.com/bad" contains characters that are not allowed.', ); }); @@ -141,7 +141,7 @@ describe('Validation Utils', () => { headers: { 'host': 'example.com?query=1' }, }); expect(() => validateRequest(req, allowedHosts, false)).toThrowError( - 'Header "host" contains characters that are not allowed.', + 'Header "host" with value "example.com?query=1" contains characters that are not allowed.', ); }); @@ -150,30 +150,17 @@ describe('Validation Utils', () => { headers: { 'x-forwarded-host': 'example.com/bad' }, }); expect(() => validateRequest(req, allowedHosts, false)).toThrowError( - 'Header "x-forwarded-host" contains characters that are not allowed.', + 'Header "x-forwarded-host" with value "example.com/bad" contains characters that are not allowed.', ); }); - it('should throw error if x-forwarded-prefix starts with a backslash or multiple slashes', () => { - const inputs = ['//evil', '\\\\evil', '/\\evil', '\\/evil', '\\evil']; - - for (const prefix of inputs) { - const request = new Request('https://example.com', { - headers: { - 'x-forwarded-prefix': prefix, - }, - }); - - expect(() => validateRequest(request, allowedHosts, false)) - .withContext(`Prefix: "${prefix}"`) - .toThrowError( - 'Header "x-forwarded-prefix" must not start with "\\" or multiple "/" or contain ".", ".." path segments.', - ); - } - }); - - it('should throw error if x-forwarded-prefix contains dot segments', () => { + it('should throw error if x-forwarded-prefix is invalid', () => { const inputs = [ + '//evil', + '\\\\evil', + '/\\evil', + '\\/evil', + '\\evil', '/./', '/../', '/foo/./bar', @@ -188,6 +175,11 @@ describe('Validation Utils', () => { '/foo/..\\bar', '.', '..', + 'https://attacker.com', + '%2f%2f', + '%5C', + 'http://localhost', + 'foo', ]; for (const prefix of inputs) { @@ -200,13 +192,14 @@ describe('Validation Utils', () => { expect(() => validateRequest(request, allowedHosts, false)) .withContext(`Prefix: "${prefix}"`) .toThrowError( - 'Header "x-forwarded-prefix" must not start with "\\" or multiple "/" or contain ".", ".." path segments.', + 'Header "x-forwarded-prefix" is invalid. It must start with a "/" and contain ' + + 'only alphanumeric characters, hyphens, and underscores, separated by single slashes.', ); } }); - it('should validate x-forwarded-prefix with valid dot usage', () => { - const inputs = ['/foo.bar', '/foo.bar/baz', '/v1.2', '/.well-known']; + it('should validate x-forwarded-prefix with valid usage', () => { + const inputs = ['/foo', '/foo/baz', '/v1', '/app_name', '/', '/my-app']; for (const prefix of inputs) { const request = new Request('https://example.com', { @@ -222,143 +215,61 @@ describe('Validation Utils', () => { }); }); - describe('cloneRequestAndPatchHeaders', () => { - const allowedHosts = new Set(['example.com', '*.valid.com']); - - it('should validate host header when accessed via get()', async () => { + describe('sanitizeRequestHeaders', () => { + it('should scrub unallowed proxy headers by default', () => { const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, + headers: { + 'host': 'example.com', + 'x-forwarded-host': 'evil.com', + 'x-forwarded-proto': 'https', + }, }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); - - expect(() => secured.headers.get('host')).toThrowError( - 'Header "host" with value "evil.com" is not allowed.', - ); - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching('Header "host" with value "evil.com" is not allowed'), - }), - ); - }); + const secured = sanitizeRequestHeaders(req); - it('should allow valid host header', () => { - const req = new Request('http://example.com', { - headers: { 'host': 'example.com' }, - }); - const { request: secured } = cloneRequestAndPatchHeaders(req, allowedHosts); expect(secured.headers.get('host')).toBe('example.com'); + expect(secured.headers.has('x-forwarded-host')).toBeFalse(); + expect(secured.headers.has('x-forwarded-proto')).toBeFalse(); }); - it('should allow any host header when "*" is used', () => { - const allowedHosts = new Set(['*']); + it('should retain allowed proxy headers when explicitly provided', () => { + const allowedProxyHeaders = new Set(['x-forwarded-host']); const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, + headers: { + 'host': 'example.com', + 'x-forwarded-host': 'proxy.com', + 'x-forwarded-proto': 'https', + }, }); - const { request: secured } = cloneRequestAndPatchHeaders(req, allowedHosts); - expect(secured.headers.get('host')).toBe('evil.com'); + const secured = sanitizeRequestHeaders(req, allowedProxyHeaders); + + expect(secured.headers.get('host')).toBe('example.com'); + expect(secured.headers.get('x-forwarded-host')).toBe('proxy.com'); + expect(secured.headers.has('x-forwarded-proto')).toBeFalse(); }); - it('should validate x-forwarded-host header', async () => { + it('should retain all proxy headers when allowedProxyHeaders is true', () => { const req = new Request('http://example.com', { - headers: { 'x-forwarded-host': 'evil.com' }, + headers: { + 'host': 'example.com', + 'x-forwarded-host': 'proxy.com', + 'x-forwarded-proto': 'https', + }, }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); + const secured = sanitizeRequestHeaders(req, true); - expect(() => secured.headers.get('x-forwarded-host')).toThrowError( - 'Header "x-forwarded-host" with value "evil.com" is not allowed.', - ); - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching( - 'Header "x-forwarded-host" with value "evil.com" is not allowed', - ), - }), - ); + expect(secured.headers.get('host')).toBe('example.com'); + expect(secured.headers.get('x-forwarded-host')).toBe('proxy.com'); + expect(secured.headers.get('x-forwarded-proto')).toBe('https'); }); - it('should allow accessing other headers without validation', () => { + it('should not clone the request if no proxy headers need to be removed', () => { const req = new Request('http://example.com', { headers: { 'accept': 'application/json' }, }); - const { request: secured } = cloneRequestAndPatchHeaders(req, allowedHosts); + const secured = sanitizeRequestHeaders(req); + expect(secured).toBe(req); expect(secured.headers.get('accept')).toBe('application/json'); }); - - it('should validate headers when iterating with entries()', async () => { - const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, - }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); - - expect(() => { - for (const _ of secured.headers.entries()) { - // access the header to trigger the validation - } - }).toThrowError('Header "host" with value "evil.com" is not allowed.'); - - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching('Header "host" with value "evil.com" is not allowed.'), - }), - ); - }); - - it('should validate headers when iterating with values()', async () => { - const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, - }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); - - expect(() => { - for (const _ of secured.headers.values()) { - // access the header to trigger the validation - } - }).toThrowError('Header "host" with value "evil.com" is not allowed.'); - - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching('Header "host" with value "evil.com" is not allowed.'), - }), - ); - }); - - it('should validate headers when iterating with for...of', async () => { - const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, - }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); - - expect(() => { - for (const _ of secured.headers) { - // access the header to trigger the validation - } - }).toThrowError('Header "host" with value "evil.com" is not allowed.'); - - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching('Header "host" with value "evil.com" is not allowed.'), - }), - ); - }); - - it('should validate headers when iterating with forEach()', async () => { - const req = new Request('http://example.com', { - headers: { 'host': 'evil.com' }, - }); - const { request: secured, onError } = cloneRequestAndPatchHeaders(req, allowedHosts); - - expect(() => { - secured.headers.forEach(() => { - // access the header to trigger the validation - }); - }).toThrowError('Header "host" with value "evil.com" is not allowed.'); - - await expectAsync(onError).toBeResolvedTo( - jasmine.objectContaining({ - message: jasmine.stringMatching('Header "host" with value "evil.com" is not allowed.'), - }), - ); - }); }); });