Prevent cache poisoning in x-forwarded headers (#14743)

* Restrict X-Forwarded-Proto and X-Forwarded-Port

* Fix X-Forwarded header security vulnerabilities

- Sanitize hostnames to reject paths and prevent path injection
- Validate X-Forwarded-Proto, X-Forwarded-Host, X-Forwarded-Port headers
- Add strict rejection for invalid hostnames (those with path separators)
- Implement single sanitizeHost() function in App class, used by both validateForwardedHeaders() and node.ts
- Add comprehensive security tests for header validation

* Fix path injection and port matching bugs in header validation

- Reject both forward and backward slashes in hostnames using single regex
- Fix allowedDomains port matching by validating full hostname:port combo instead of just hostname
- Add test for X-Forwarded-Host with embedded port in allowedDomains pattern

* changeset and build

* fix: validate X-Forwarded headers with port pattern matching

Fixes protocol validation to accept http/https when allowedDomains exist but lack protocol patterns. Restructures port/host validation to validate port first, then include it when validating host against patterns. Properly extracts hostname without port to avoid duplication when combining with X-Forwarded-Port.

* Update .changeset/secure-headers.md

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>

---------

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>

.changeset/secure-headers.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Improves `X-Forwarded` header validation to prevent cache poisoning and header injection attacks. Now properly validates `X-Forwarded-Proto`, `X-Forwarded-Host`, and `X-Forwarded-Port` headers against configured `allowedDomains` patterns, rejecting malformed or suspicious values. This is especially important when running behind a reverse proxy or load balancer.

packages/astro/src/core/app/index.ts

@@ -174,6 +174,98 @@ export class App {
 		}
 	}
 
+	/**
+	 * Validate a hostname by rejecting any with path separators.
+	 * Prevents path injection attacks. Invalid hostnames return undefined.
+	 */
+	static sanitizeHost(hostname: string | undefined): string | undefined {
+		if (!hostname) return undefined;
+		// Reject any hostname containing path separators - they're invalid
+		if (/[/\\]/.test(hostname)) return undefined;
+		return hostname;
+	}
+
+	/**
+	 * Validate forwarded headers (proto, host, port) against allowedDomains.
+	 * Returns validated values or undefined for rejected headers.
+	 * Uses strict defaults: http/https only for proto, rejects port if not in allowedDomains.
+	 */
+	static validateForwardedHeaders(
+		forwardedProtocol?: string,
+		forwardedHost?: string,
+		forwardedPort?: string,
+		allowedDomains?: Partial<RemotePattern>[],
+	): { protocol?: string; host?: string; port?: string } {
+		const result: { protocol?: string; host?: string; port?: string } = {};
+
+		// Validate protocol
+		if (forwardedProtocol) {
+			if (allowedDomains && allowedDomains.length > 0) {
+				const hasProtocolPatterns = allowedDomains.some(
+					(pattern) => pattern.protocol !== undefined,
+				);
+				if (hasProtocolPatterns) {
+					// Validate against allowedDomains patterns
+					try {
+						const testUrl = new URL(`${forwardedProtocol}://example.com`);
+						const isAllowed = allowedDomains.some((pattern) => matchPattern(testUrl, pattern));
+						if (isAllowed) {
+							result.protocol = forwardedProtocol;
+						}
+					} catch {
+						// Invalid protocol, omit from result
+					}
+				} else if (/^https?$/.test(forwardedProtocol)) {
+					// allowedDomains exist but no protocol patterns, allow http/https
+					result.protocol = forwardedProtocol;
+				}
+			} else if (/^https?$/.test(forwardedProtocol)) {
+				// No allowedDomains, only allow http/https
+				result.protocol = forwardedProtocol;
+			}
+		}
+
+		// Validate port first
+		if (forwardedPort && allowedDomains && allowedDomains.length > 0) {
+			const hasPortPatterns = allowedDomains.some((pattern) => pattern.port !== undefined);
+			if (hasPortPatterns) {
+				// Validate against allowedDomains patterns
+				const isAllowed = allowedDomains.some((pattern) => pattern.port === forwardedPort);
+				if (isAllowed) {
+					result.port = forwardedPort;
+				}
+			}
+			// If no port patterns, reject the header (strict security default)
+		}
+
+		// Validate host (extract port from hostname for validation)
+		// Reject empty strings and sanitize to prevent path injection
+		if (forwardedHost && forwardedHost.length > 0 && allowedDomains && allowedDomains.length > 0) {
+			const protoForValidation = result.protocol || 'https';
+			const sanitized = App.sanitizeHost(forwardedHost);
+			if (sanitized) {
+				try {
+					// Extract hostname without port for validation
+					const hostnameOnly = sanitized.split(':')[0];
+					// Use full hostname:port for validation so patterns with ports match correctly
+					// Include validated port if available, otherwise use port from forwardedHost if present
+					const portFromHost = sanitized.includes(':') ? sanitized.split(':')[1] : undefined;
+					const portForValidation = result.port || portFromHost;
+					const hostWithPort = portForValidation ? `${hostnameOnly}:${portForValidation}` : hostnameOnly;
+					const testUrl = new URL(`${protoForValidation}://${hostWithPort}`);
+					const isAllowed = allowedDomains.some((pattern) => matchPattern(testUrl, pattern));
+					if (isAllowed) {
+						result.host = sanitized;
+					}
+				} catch {
+					// Invalid host, omit from result
+				}
+			}
+		}
+
+		return result;
+	}
+
 	/**
 	 * Creates a pipeline by reading the stored manifest
 	 *
@@ -271,29 +363,19 @@ export class App {
 				this.#manifest.i18n.strategy === 'domains-prefix-other-locales' ||
 				this.#manifest.i18n.strategy === 'domains-prefix-always-no-redirect')
 		) {
-			// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
-			let forwardedHost = request.headers.get('X-Forwarded-Host');
-			// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
-			let protocol = request.headers.get('X-Forwarded-Proto');
-			if (protocol) {
-				// this header doesn't have a colon at the end, so we add to be in line with URL#protocol, which does have it
-				protocol = protocol + ':';
-			} else {
-				// we fall back to the protocol of the request
-				protocol = url.protocol;
-			}
+			// Validate forwarded headers
+			const validated = App.validateForwardedHeaders(
+				request.headers.get('X-Forwarded-Proto') ?? undefined,
+				request.headers.get('X-Forwarded-Host') ?? undefined,
+				request.headers.get('X-Forwarded-Port') ?? undefined,
+				this.#manifest.allowedDomains,
+			);
 
-			// Validate X-Forwarded-Host against allowedDomains if configured
-			if (forwardedHost && !this.matchesAllowedDomains(forwardedHost, protocol?.replace(':', ''))) {
-				// If not allowed, ignore the X-Forwarded-Host header
-				forwardedHost = null;
-			}
+			// Build protocol with fallback
+			let protocol = validated.protocol ? validated.protocol + ':' : url.protocol;
 
-			let host = forwardedHost;
-			if (!host) {
-				// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host
-				host = request.headers.get('Host');
-			}
+			// Build host with fallback
+			let host = validated.host ?? request.headers.get('Host');
 			// If we don't have a host and a protocol, it's impossible to proceed
 			if (host && protocol) {
 				// The header might have a port in their name, so we remove it

packages/astro/src/core/app/node.ts

@@ -2,7 +2,6 @@ import fs from 'node:fs';
 import type { IncomingMessage, ServerResponse } from 'node:http';
 import { Http2ServerResponse } from 'node:http2';
 import type { Socket } from 'node:net';
-// matchPattern is used in App.validateForwardedHost, no need to import here
 import type { RemotePattern } from '../../types/public/config.js';
 import type { RouteData } from '../../types/public/internal.js';
 import { clientAddressSymbol, nodeRequestAbortControllerCleanupSymbol } from '../constants.js';
@@ -90,35 +89,25 @@ export class NodeApp extends App {
 				.map((e) => e.trim())?.[0];
 		};
 
-		// Get the used protocol between the end client and first proxy.
-		// NOTE: Some proxies append values with spaces and some do not.
-		// We need to handle it here and parse the header correctly.
-		// @example "https, http,http" => "http"
-		const forwardedProtocol = getFirstForwardedValue(req.headers['x-forwarded-proto']);
 		const providedProtocol = isEncrypted ? 'https' : 'http';
-		const protocol = forwardedProtocol ?? providedProtocol;
-
-		// @example "example.com,www2.example.com" => "example.com"
-		let forwardedHostname = getFirstForwardedValue(req.headers['x-forwarded-host']);
 		const providedHostname = req.headers.host ?? req.headers[':authority'];
 
-		// Validate X-Forwarded-Host against allowedDomains if configured
-		if (
-			forwardedHostname &&
-			!App.validateForwardedHost(
-				forwardedHostname,
-				allowedDomains,
-				forwardedProtocol ?? providedProtocol,
-			)
-		) {
-			// If not allowed, ignore the X-Forwarded-Host header
-			forwardedHostname = undefined;
-		}
+		// Validate forwarded headers
+		// NOTE: Header values may have commas/spaces from proxy chains, extract first value
+		const validated = App.validateForwardedHeaders(
+			getFirstForwardedValue(req.headers['x-forwarded-proto']),
+			getFirstForwardedValue(req.headers['x-forwarded-host']),
+			getFirstForwardedValue(req.headers['x-forwarded-port']),
+			allowedDomains,
+		);
 
-		const hostname = forwardedHostname ?? providedHostname;
-
-		// @example "443,8080,80" => "443"
-		const port = getFirstForwardedValue(req.headers['x-forwarded-port']);
+		const protocol = validated.protocol ?? providedProtocol;
+		// validated.host is already sanitized, only sanitize providedHostname
+		const sanitizedProvidedHostname = App.sanitizeHost(
+			typeof providedHostname === 'string' ? providedHostname : undefined,
+		);
+		const hostname = validated.host ?? sanitizedProvidedHostname;
+		const port = validated.port;
 
 		let url: URL;
 		try {

packages/astro/test/units/app/node.test.js

@@ -104,6 +104,102 @@ describe('NodeApp', () => {
 				});
 				assert.equal(result.url, 'https://example.com/');
 			});
+
+			it('rejects empty x-forwarded-host and falls back to host header', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'legitimate.example.com',
+							'x-forwarded-host': '',
+						},
+					},
+					{ allowedDomains: [{ hostname: 'example.com' }] },
+				);
+				assert.equal(result.url, 'https://legitimate.example.com/');
+			});
+
+			it('rejects x-forwarded-host with path separator (path injection attempt)', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-host': 'example.com/admin',
+						},
+					},
+					{ allowedDomains: [{ hostname: 'example.com', protocol: 'https' }] },
+				);
+				// Path separator in host is rejected, falls back to Host header
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects x-forwarded-host with multiple path segments', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-host': 'example.com/admin/users',
+						},
+					},
+					{ allowedDomains: [{ hostname: 'example.com', protocol: 'https' }] },
+				);
+				// Path separators in host are rejected, falls back to Host header
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects x-forwarded-host with backslash path separator (path injection attempt)', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-host': 'example.com\\admin',
+						},
+					},
+					{ allowedDomains: [{ hostname: 'example.com', protocol: 'https' }] },
+				);
+				// Backslash separator in host is rejected, falls back to Host header
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('parses x-forwarded-host with embedded port when allowedDomains has port pattern', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-host': 'example.com:3000',
+						},
+					},
+					{ allowedDomains: [{ hostname: 'example.com', port: '3000' }] },
+				);
+				// X-Forwarded-Host with port should match pattern that includes port
+				assert.equal(result.url, 'https://example.com:3000/');
+			});
+		});
+
+		it('rejects Host header with path separator (path injection attempt)', () => {
+			const result = NodeApp.createRequest({
+				...mockNodeRequest,
+				headers: {
+					host: 'example.com/admin',
+				},
+			});
+			// Host header with path is rejected, resulting in undefined hostname
+			assert.equal(result.url, 'https://undefined/');
+		});
+
+		it('rejects Host header with backslash path separator (path injection attempt)', () => {
+			const result = NodeApp.createRequest({
+				...mockNodeRequest,
+				headers: {
+					host: 'example.com\\admin',
+				},
+			});
+			// Host header with backslash is rejected, resulting in undefined hostname
+			assert.equal(result.url, 'https://undefined/');
 		});
 
 		describe('x-forwarded-proto', () => {
@@ -140,10 +236,93 @@ describe('NodeApp', () => {
 				});
 				assert.equal(result.url, 'https://example.com/');
 			});
+
+			it('rejects malicious x-forwarded-proto with URL injection (https://www.malicious-url.com/?tank=)', () => {
+				const result = NodeApp.createRequest({
+					...mockNodeRequest,
+					headers: {
+						host: 'example.com',
+						'x-forwarded-proto': 'https://www.malicious-url.com/?tank=',
+					},
+				});
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects malicious x-forwarded-proto with middleware bypass attempt (x:admin?)', () => {
+				const result = NodeApp.createRequest({
+					...mockNodeRequest,
+					headers: {
+						host: 'example.com',
+						'x-forwarded-proto': 'x:admin?',
+					},
+				});
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects malicious x-forwarded-proto with cache poison attempt (https://localhost/vulnerable?)', () => {
+				const result = NodeApp.createRequest({
+					...mockNodeRequest,
+					headers: {
+						host: 'example.com',
+						'x-forwarded-proto': 'https://localhost/vulnerable?',
+					},
+				});
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects malicious x-forwarded-proto with XSS attempt (javascript:alert(document.cookie)//)', () => {
+				const result = NodeApp.createRequest({
+					...mockNodeRequest,
+					headers: {
+						host: 'example.com',
+						'x-forwarded-proto': 'javascript:alert(document.cookie)//',
+					},
+				});
+				assert.equal(result.url, 'https://example.com/');
+			});
+
+			it('rejects empty x-forwarded-proto and falls back to encrypted property', () => {
+				const result = NodeApp.createRequest({
+					...mockNodeRequest,
+					headers: {
+						host: 'example.com',
+						'x-forwarded-proto': '',
+					},
+				});
+				assert.equal(result.url, 'https://example.com/');
+			});
 		});
 
 		describe('x-forwarded-port', () => {
-			it('parses port from single-value x-forwarded-port header', () => {
+			it('parses port from single-value x-forwarded-port header (with allowedDomains)', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-port': '8443',
+						},
+					},
+					{ allowedDomains: [{ port: '8443' }] },
+				);
+				assert.equal(result.url, 'https://example.com:8443/');
+			});
+
+			it('parses port from multi-value x-forwarded-port header (with allowedDomains)', () => {
+				const result = NodeApp.createRequest(
+					{
+						...mockNodeRequest,
+						headers: {
+							host: 'example.com',
+							'x-forwarded-port': '8443,3000',
+						},
+					},
+					{ allowedDomains: [{ port: '8443' }] },
+				);
+				assert.equal(result.url, 'https://example.com:8443/');
+			});
+
+			it('rejects x-forwarded-port without allowedDomains patterns (strict security default)', () => {
 				const result = NodeApp.createRequest({
 					...mockNodeRequest,
 					headers: {
@@ -151,18 +330,7 @@ describe('NodeApp', () => {
 						'x-forwarded-port': '8443',
 					},
 				});
-				assert.equal(result.url, 'https://example.com:8443/');
-			});
-
-			it('parses port from multi-value x-forwarded-port header', () => {
-				const result = NodeApp.createRequest({
-					...mockNodeRequest,
-					headers: {
-						host: 'example.com',
-						'x-forwarded-port': '8443,3000',
-					},
-				});
-				assert.equal(result.url, 'https://example.com:8443/');
+				assert.equal(result.url, 'https://example.com/');
 			});
 
 			it('prefers port from host', () => {
@@ -176,14 +344,13 @@ describe('NodeApp', () => {
 				assert.equal(result.url, 'https://example.com:3000/');
 			});
 
-			it('prefers port from x-forwarded-host', () => {
+			it('uses port embedded in x-forwarded-host', () => {
 				const result = NodeApp.createRequest(
 					{
 						...mockNodeRequest,
 						headers: {
-							host: 'example.com:443',
+							host: 'example.com',
 							'x-forwarded-host': 'example.com:3000',
-							'x-forwarded-port': '443',
 						},
 					},
 					{ allowedDomains: [{ hostname: 'example.com' }] },

packages/integrations/node/test/fixtures/url/astro.config.mjs

@@ -7,7 +7,8 @@ export default defineConfig({
 	security: {
 		allowedDomains: [
 			{
-				hostname: 'abc.xyz'
+				hostname: 'abc.xyz',
+				port: '444'
 			}
 		]
 	}

packages/integrations/node/test/url.test.js

@@ -135,7 +135,7 @@ describe('URL', () => {
 		assert.equal($('body').text(), 'https://legitimate.example.com/');
 	});
 
-	it('accepts any port when port not specified in allowedDomains', async () => {
+	it('rejects port in forwarded host when port not in allowedDomains', async () => {
 		const { handler } = await import('./fixtures/url/dist/server/entry.mjs');
 		const { req, res, text } = createRequestAndResponse({
 			headers: {
@@ -152,8 +152,49 @@ describe('URL', () => {
 		const html = await text();
 		const $ = cheerio.load(html);
 
-		// When no port is specified in allowedDomains pattern, any port is accepted
-		// This validates that port validation works (it's checking and passing)
-		assert.equal($('body').text(), 'https://abc.xyz:8080/');
+		// Port 8080 not in allowedDomains (only 444), so should fall back to Host header
+		assert.equal($('body').text(), 'https://localhost:3000/');
+	});
+
+	it('rejects empty X-Forwarded-Host with allowedDomains configured', async () => {
+		const { handler } = await import('./fixtures/url/dist/server/entry.mjs');
+		const { req, res, text } = createRequestAndResponse({
+			headers: {
+				'X-Forwarded-Proto': 'https',
+				'X-Forwarded-Host': '',
+				Host: 'legitimate.example.com',
+			},
+			url: '/',
+		});
+
+		handler(req, res);
+		req.send();
+
+		const html = await text();
+		const $ = cheerio.load(html);
+
+		// Empty X-Forwarded-Host should be rejected and fall back to Host header
+		assert.equal($('body').text(), 'https://legitimate.example.com/');
+	});
+
+	it('rejects X-Forwarded-Host with path injection attempt', async () => {
+		const { handler } = await import('./fixtures/url/dist/server/entry.mjs');
+		const { req, res, text } = createRequestAndResponse({
+			headers: {
+				'X-Forwarded-Proto': 'https',
+				'X-Forwarded-Host': 'example.com/admin',
+				Host: 'localhost:3000',
+			},
+			url: '/',
+		});
+
+		handler(req, res);
+		req.send();
+
+		const html = await text();
+		const $ = cheerio.load(html);
+
+		// Path injection attempt should be rejected and fall back to Host header
+		assert.equal($('body').text(), 'https://localhost:3000/');
 	});
 });