Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions packages/angular/ssr/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,9 @@
*/

/**
* Common X-Forwarded-* headers.
* Internal sentinel string representing a wildcard rule to trust all proxy headers.
*/
const X_FORWARDED_HEADERS: ReadonlySet<string> = new Set([
'x-forwarded-for',
'x-forwarded-host',
'x-forwarded-port',
'x-forwarded-proto',
'x-forwarded-prefix',
]);
const TRUST_ALL_PROXY_HEADERS = 'ɵ*';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is there a particular reason to not use *? Are we worried users might explicitly pass in * rather than using true? Would it be safer to validate that input rather than use a ɵ character?


/**
* The set of headers that should be validated for host header injection attacks.
Expand Down Expand Up @@ -235,7 +229,10 @@ export function isProxyHeaderAllowed(
headerName: string,
trustProxyHeaders: ReadonlySet<string>,
): boolean {
return trustProxyHeaders.has(headerName.toLowerCase());
return (
trustProxyHeaders.has(TRUST_ALL_PROXY_HEADERS) ||
trustProxyHeaders.has(headerName.toLowerCase())
);
}

/**
Expand All @@ -251,7 +248,7 @@ export function normalizeTrustProxyHeaders(
}

if (trustProxyHeaders === true) {
return X_FORWARDED_HEADERS;
return new Set([TRUST_ALL_PROXY_HEADERS]);
}

return new Set(trustProxyHeaders.map((h) => h.toLowerCase()));
Expand Down
32 changes: 23 additions & 9 deletions packages/angular/ssr/test/utils/validation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import {
getFirstHeaderValue,
normalizeTrustProxyHeaders,
sanitizeRequestHeaders,
validateRequest,
validateUrl,
Expand Down Expand Up @@ -224,23 +225,37 @@ describe('Validation Utils', () => {
'x-forwarded-proto': 'https',
},
});
const secured = sanitizeRequestHeaders(req, new Set());
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(undefined));

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 retain allowed proxy headers when explicitly provided', () => {
const trustProxyHeaders = new Set(['x-forwarded-host']);
it('should scrub unallowed proxy headers when trustProxyHeaders is false', () => {
const req = new Request('http://example.com', {
headers: {
'host': 'example.com',
'x-forwarded-host': 'evil.com',
'x-forwarded-proto': 'https',
},
});
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(false));

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 only retain allowed proxy headers when explicitly provided', () => {
const req = new Request('http://example.com', {
headers: {
'host': 'example.com',
'x-forwarded-host': 'proxy.com',
'x-forwarded-proto': 'https',
},
});
const secured = sanitizeRequestHeaders(req, trustProxyHeaders);
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(['x-forwarded-host']));

expect(secured.headers.get('host')).toBe('example.com');
expect(secured.headers.get('x-forwarded-host')).toBe('proxy.com');
Expand All @@ -253,23 +268,22 @@ describe('Validation Utils', () => {
'host': 'example.com',
'x-forwarded-host': 'proxy.com',
'x-forwarded-proto': 'https',
'x-forwarded-email': 'user@example.com',
},
});
const secured = sanitizeRequestHeaders(
req,
new Set(['x-forwarded-host', 'x-forwarded-proto']),
);
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(true));

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');
expect(secured.headers.get('x-forwarded-email')).toBe('user@example.com');
});

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 secured = sanitizeRequestHeaders(req, new Set());
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(false));

expect(secured).toBe(req);
expect(secured.headers.get('accept')).toBe('application/json');
Expand Down
Loading