diff --git a/.changeset/harden-xff-allowed-domains.md b/.changeset/harden-xff-allowed-domains.md new file mode 100644 index 000000000000..655133d29546 --- /dev/null +++ b/.changeset/harden-xff-allowed-domains.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hardens `clientAddress` resolution to respect `security.allowedDomains` for `X-Forwarded-For`, consistent with the existing handling of `X-Forwarded-Host`, `X-Forwarded-Proto`, and `X-Forwarded-Port`. The `X-Forwarded-For` header is now only used to determine `Astro.clientAddress` when the request's host has been validated against an `allowedDomains` entry. Without a matching domain, `clientAddress` falls back to the socket's remote address. diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 716ec5205f76..b58db09ec2eb 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -141,8 +141,14 @@ export function createRequest( } // Get the IP of end client behind the proxy. + // Only trust X-Forwarded-For when the request's host was validated against allowedDomains, + // meaning it arrived through a trusted proxy. Without this check, any client can spoof + // their IP via this header. // @example "1.1.1.1,8.8.8.8" => "1.1.1.1" - const forwardedClientIp = getFirstForwardedValue(req.headers['x-forwarded-for']); + const hostValidated = validated.host !== undefined || validatedHostname !== undefined; + const forwardedClientIp = hostValidated + ? getFirstForwardedValue(req.headers['x-forwarded-for']) + : undefined; const clientIp = forwardedClientIp || req.socket?.remoteAddress; if (clientIp) { Reflect.set(request, clientAddressSymbol, clientIp); diff --git a/packages/astro/test/fixtures/client-address-node/astro.config.mjs b/packages/astro/test/fixtures/client-address-node/astro.config.mjs index ad9b3a3b16df..d48e21837c5f 100644 --- a/packages/astro/test/fixtures/client-address-node/astro.config.mjs +++ b/packages/astro/test/fixtures/client-address-node/astro.config.mjs @@ -5,4 +5,7 @@ import { defineConfig } from 'astro/config'; export default defineConfig({ output: 'server', adapter: node({ mode: 'middleware' }), + security: { + allowedDomains: [{ hostname: 'localhost' }], + }, }); diff --git a/packages/astro/test/units/app/node.test.js b/packages/astro/test/units/app/node.test.js index 213408915ef5..a7a8d5c9909d 100644 --- a/packages/astro/test/units/app/node.test.js +++ b/packages/astro/test/units/app/node.test.js @@ -19,40 +19,186 @@ describe('node', () => { describe('createRequest', () => { describe('x-forwarded-for', () => { it('parses client IP from single-value x-forwarded-for header', () => { - const result = createRequest({ - ...mockNodeRequest, - headers: { - 'x-forwarded-for': '1.1.1.1', + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': '1.1.1.1', + }, }, - }); + { allowedDomains: [{ hostname: 'example.com' }] }, + ); assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); }); it('parses client IP from multi-value x-forwarded-for header', () => { - const result = createRequest({ - ...mockNodeRequest, - headers: { - 'x-forwarded-for': '1.1.1.1,8.8.8.8', + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': '1.1.1.1,8.8.8.8', + }, }, - }); + { allowedDomains: [{ hostname: 'example.com' }] }, + ); assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); }); it('parses client IP from multi-value x-forwarded-for header with spaces', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': ' 1.1.1.1, 8.8.8.8, 8.8.8.2', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('fallbacks to remoteAddress when no x-forwarded-for header is present', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('ignores x-forwarded-for when no allowedDomains is configured (default)', () => { const result = createRequest({ ...mockNodeRequest, headers: { - 'x-forwarded-for': ' 1.1.1.1, 8.8.8.8, 8.8.8.2', + host: 'example.com', + 'x-forwarded-for': '1.1.1.1', }, }); + // Without allowedDomains, x-forwarded-for should NOT be trusted + // Falls back to socket remoteAddress + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('ignores x-forwarded-for when allowedDomains is empty', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': '1.1.1.1', + }, + }, + { allowedDomains: [] }, + ); + // Empty allowedDomains means no proxy trust, use socket address + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('trusts x-forwarded-for when host matches allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': '1.1.1.1', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + // Host matches allowedDomains, so x-forwarded-for is trusted assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); }); - it('fallbacks to remoteAddress when no x-forwarded-for header is present', () => { + it('ignores x-forwarded-for when host does not match allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'attacker.com', + 'x-forwarded-for': '1.1.1.1', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + // Host does not match allowedDomains, so x-forwarded-for is NOT trusted + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('trusts x-forwarded-for when x-forwarded-host matches allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + 'x-forwarded-host': 'example.com', + 'x-forwarded-for': '1.1.1.1', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + // X-Forwarded-Host validated against allowedDomains, so XFF is trusted + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('trusts multi-value x-forwarded-for when host matches allowedDomains', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + 'x-forwarded-for': '1.1.1.1, 8.8.8.8', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + assert.equal(result[Symbol.for('astro.clientAddress')], '1.1.1.1'); + }); + + it('falls back to remoteAddress when host matches allowedDomains but no x-forwarded-for', () => { + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'example.com', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('prevents IP spoofing: attacker cannot override clientAddress without allowedDomains', () => { + // Simulates an attacker injecting x-forwarded-for to spoof 127.0.0.1 const result = createRequest({ ...mockNodeRequest, - headers: {}, + headers: { + host: 'example.com', + 'x-forwarded-for': '127.0.0.1', + }, }); + // Without allowedDomains, the spoofed IP must be ignored + assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); + }); + + it('prevents IP spoofing: attacker cannot override clientAddress when host does not match', () => { + // Simulates attacker sending direct request with XFF and mismatched host + const result = createRequest( + { + ...mockNodeRequest, + headers: { + host: 'evil.com', + 'x-forwarded-for': '127.0.0.1', + }, + }, + { allowedDomains: [{ hostname: 'example.com' }] }, + ); + // Host doesn't match allowedDomains, so XFF is not trusted assert.equal(result[Symbol.for('astro.clientAddress')], '2.2.2.2'); }); });