diff --git a/.changeset/secure-headers.md b/.changeset/secure-headers.md new file mode 100644 index 000000000000..bb7ff1db15af --- /dev/null +++ b/.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. diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 47592ffd5e2e..be8f895a9529 100644 --- a/packages/astro/src/core/app/index.ts +++ b/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[], + ): { 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 diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index c70375accb45..6da81841d67e 100644 --- a/packages/astro/src/core/app/node.ts +++ b/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; - } - - const hostname = forwardedHostname ?? providedHostname; + // 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, + ); - // @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 { diff --git a/packages/astro/test/units/app/node.test.js b/packages/astro/test/units/app/node.test.js index ad1ddf435b41..f94c88238d2a 100644 --- a/packages/astro/test/units/app/node.test.js +++ b/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,31 +236,103 @@ describe('NodeApp', () => { }); assert.equal(result.url, 'https://example.com/'); }); - }); - describe('x-forwarded-port', () => { - it('parses port from single-value x-forwarded-port header', () => { + 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-port': '8443', + 'x-forwarded-proto': 'https://www.malicious-url.com/?tank=', }, }); - assert.equal(result.url, 'https://example.com:8443/'); + 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('parses port from multi-value x-forwarded-port header', () => { + it('rejects malicious x-forwarded-proto with cache poison attempt (https://localhost/vulnerable?)', () => { const result = NodeApp.createRequest({ ...mockNodeRequest, headers: { host: 'example.com', - 'x-forwarded-port': '8443,3000', + '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 (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: { + host: 'example.com', + 'x-forwarded-port': '8443', + }, + }); + assert.equal(result.url, 'https://example.com/'); + }); + it('prefers port from host', () => { const result = NodeApp.createRequest({ ...mockNodeRequest, @@ -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' }] }, diff --git a/packages/integrations/node/test/fixtures/url/astro.config.mjs b/packages/integrations/node/test/fixtures/url/astro.config.mjs index 0f17f53877c0..7aeda667af91 100644 --- a/packages/integrations/node/test/fixtures/url/astro.config.mjs +++ b/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' } ] } diff --git a/packages/integrations/node/test/url.test.js b/packages/integrations/node/test/url.test.js index 9295210e085b..c3e90ead56f9 100644 --- a/packages/integrations/node/test/url.test.js +++ b/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/'); }); });