Skip to content
Closed
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
4 changes: 2 additions & 2 deletions packages/angular/ssr/src/app-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
*/
export interface AngularAppEngineOptions {
/**
* A set of allowed hostnames for the server application.
* A set of allowed hosts for the server application.
*/
allowedHosts?: readonly string[];

Expand Down Expand Up @@ -90,7 +90,7 @@ export class AngularAppEngine {
private readonly manifest = getAngularAppEngineManifest();

/**
* A set of allowed hostnames for the server application.
* A set of allowed hosts for the server application.
*/
private readonly allowedHosts: ReadonlySet<string>;

Expand Down
2 changes: 1 addition & 1 deletion packages/angular/ssr/src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export interface AngularAppEngineManifest {
readonly supportedLocales: Readonly<Record<string, string>>;

/**
* A readonly array of allowed hostnames.
* A readonly array of allowed hosts.
*/
readonly allowedHosts: Readonly<string[]>;
}
Expand Down
74 changes: 47 additions & 27 deletions packages/angular/ssr/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function getFirstHeaderValue(
* Validates a request.
*
* @param request - The incoming `Request` object to validate.
* @param allowedHosts - A set of allowed hostnames.
* @param allowedHosts - A set of allowed hosts.
* @param disableHostCheck - Whether to disable the host check.
* @throws Error if any of the validated headers contain invalid values.
*/
Expand All @@ -71,24 +71,24 @@ export function validateRequest(
allowedHosts: ReadonlySet<string>,
disableHostCheck: boolean,
): void {
validateHeaders(request, allowedHosts, disableHostCheck);
const url = new URL(request.url);
validateHeaders(request, allowedHosts, disableHostCheck, url.protocol);

if (!disableHostCheck) {
validateUrl(new URL(request.url), allowedHosts);
validateUrl(url, allowedHosts);
}
}

/**
* Validates that the hostname of a given URL is allowed.
* Validates that the host of a given URL is allowed.
*
* @param url - The URL object to validate.
* @param allowedHosts - A set of allowed hostnames.
* @throws Error if the hostname is not in the allowlist.
* @param allowedHosts - A set of allowed hosts.
* @throws Error if the host is not in the allowlist.
*/
export function validateUrl(url: URL, allowedHosts: ReadonlySet<string>): void {
const { hostname } = url;
if (!isHostAllowed(hostname, allowedHosts)) {
throw new Error(`URL with hostname "${hostname}" is not allowed.`);
if (!isHostAllowed(url, allowedHosts)) {
throw new Error(`URL with host "${url.host}" is not allowed.`);
}
}

Expand Down Expand Up @@ -134,74 +134,94 @@ export function sanitizeRequestHeaders(
*
* @param headerName - The name of the header to validate (e.g., 'host', 'x-forwarded-host').
* @param headerValue - The value of the header to validate.
* @param allowedHosts - A set of allowed hostnames.
* @param allowedHosts - A set of allowed hosts.
* @throws Error if the header value is invalid or the hostname is not in the allowlist.
*/
function verifyHostAllowed(
headerName: string,
headerValue: string,
allowedHosts: ReadonlySet<string>,
protocol: string,
): void {
const url = `http://${headerValue}`;
const url = `${protocol}//${headerValue}`;
if (!URL.canParse(url)) {
throw new Error(`Header "${headerName}" contains an invalid value and cannot be parsed.`);
}

const { hostname, pathname, search, hash, username, password } = new URL(url);
const parsedUrl = new URL(url);
const { pathname, search, hash, username, password } = parsedUrl;
if (pathname !== '/' || search || hash || username || password) {
throw new Error(
`Header "${headerName}" with value "${headerValue}" contains characters that are not allowed.`,
);
}

if (!isHostAllowed(hostname, allowedHosts)) {
if (!isHostAllowed(parsedUrl, allowedHosts)) {
throw new Error(`Header "${headerName}" with value "${headerValue}" is not allowed.`);
}
}

/**
* Checks if the hostname is allowed.
* @param hostname - The hostname to check.
* @param allowedHosts - A set of allowed hostnames.
* @returns `true` if the hostname is allowed, `false` otherwise.
* Checks if the host is allowed.
* @param url - The URL to check.
* @param allowedHosts - A set of allowed hosts.
* @returns `true` if the host is allowed, `false` otherwise.
*/
function isHostAllowed(hostname: string, allowedHosts: ReadonlySet<string>): boolean {
if (allowedHosts.has('*') || allowedHosts.has(hostname)) {
function isHostAllowed(url: URL, allowedHosts: ReadonlySet<string>): boolean {
if (allowedHosts.has('*') || allowedHosts.has(url.host)) {
return true;
}

for (const allowedHost of allowedHosts) {
if (!allowedHost.startsWith('*.')) {
continue;
}

const domain = allowedHost.slice(1);
if (hostname.endsWith(domain)) {
if (isAllowedHostMatch(url, allowedHost)) {
return true;
}
}

return false;
}
Comment on lines +170 to 182
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current implementation of isHostAllowed removes the fast path for exact matches that existed in the previous version. Since isHostAllowed is called for every request and iterates through the allowedHosts set—performing expensive URL parsing in each iteration via isAllowedHostMatch—adding back a fast path for exact matches of the normalized host (including the port) would significantly improve performance for the most common cases.

Using url.host in the has check is appropriate here as it includes the port for non-default values and excludes it for default ones, matching the expected normalization.

Suggested change
function isHostAllowed(url: URL, allowedHosts: ReadonlySet<string>): boolean {
if (allowedHosts.has('*')) {
return true;
}
for (const allowedHost of allowedHosts) {
if (!allowedHost.startsWith('*.')) {
continue;
}
const domain = allowedHost.slice(1);
if (hostname.endsWith(domain)) {
if (isAllowedHostMatch(url, allowedHost)) {
return true;
}
}
return false;
}
function isHostAllowed(url: URL, allowedHosts: ReadonlySet<string>): boolean {
if (allowedHosts.has('*') || allowedHosts.has(url.host)) {
return true;
}
for (const allowedHost of allowedHosts) {
if (isAllowedHostMatch(url, allowedHost)) {
return true;
}
}
return false;
}


function isAllowedHostMatch(url: URL, allowedHost: string): boolean {
const wildcard = allowedHost.startsWith('*.');
const comparableAllowedHost = wildcard ? `placeholder${allowedHost.slice(1)}` : allowedHost;
const allowedUrl = `${url.protocol}//${comparableAllowedHost}`;

if (!URL.canParse(allowedUrl)) {
return false;
}

const parsedAllowedUrl = new URL(allowedUrl);
if (url.port !== parsedAllowedUrl.port) {
return false;
}

if (wildcard) {
const domain = parsedAllowedUrl.hostname.slice('placeholder'.length);
return url.hostname.endsWith(domain);
}

return url.hostname === parsedAllowedUrl.hostname;
}

/**
* 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 allowedHosts - A set of allowed hosts.
* @param disableHostCheck - Whether to disable the host check.
* @throws Error if any of the validated headers contain invalid values.
*/
function validateHeaders(
request: Request,
allowedHosts: ReadonlySet<string>,
disableHostCheck: boolean,
protocol: string,
): void {
const headers = request.headers;
for (const headerName of HOST_HEADERS_TO_VALIDATE) {
const headerValue = getFirstHeaderValue(headers.get(headerName));
if (headerValue && !disableHostCheck) {
verifyHostAllowed(headerName, headerValue, allowedHosts);
verifyHostAllowed(headerName, headerValue, allowedHosts, protocol);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/angular/ssr/test/app-engine_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,9 @@ describe('AngularAppEngine', () => {
const response = await appEngine.handle(request);
expect(response).not.toBeNull();
expect(response?.status).toBe(400);
expect(await response?.text()).toContain('URL with hostname "evil.com" is not allowed.');
expect(await response?.text()).toContain('URL with host "evil.com" is not allowed.');
expect(consoleErrorSpy).toHaveBeenCalledWith(
jasmine.stringMatching('URL with hostname "evil.com" is not allowed.'),
jasmine.stringMatching('URL with host "evil.com" is not allowed.'),
);
});

Expand Down
60 changes: 57 additions & 3 deletions packages/angular/ssr/test/utils/validation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Validation Utils', () => {

it('should throw for disallowed hostname', () => {
expect(() => validateUrl(new URL('http://evil.com'), allowedHosts)).toThrowError(
/URL with hostname "evil.com" is not allowed/,
/URL with host "evil.com" is not allowed/,
);
});

Expand All @@ -61,7 +61,7 @@ describe('Validation Utils', () => {
it('should not match base domain for wildcard (*.google.com vs google.com)', () => {
// Logic: hostname.endsWith('.google.com') -> 'google.com'.endsWith('.google.com') is false
expect(() => validateUrl(new URL('http://google.com'), allowedHosts)).toThrowError(
/URL with hostname "google.com" is not allowed/,
/URL with host "google.com" is not allowed/,
);
});

Expand All @@ -71,6 +71,33 @@ describe('Validation Utils', () => {
expect(() => validateUrl(new URL('http://google.com'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('http://evil.com'), allowedHosts)).not.toThrow();
});

it('should reject arbitrary ports on an allowed hostname', () => {
expect(() => validateUrl(new URL('http://example.com:8080'), allowedHosts)).toThrowError(
/URL with host "example.com:8080" is not allowed/,
);
});

it('should pass for default ports on an allowed hostname', () => {
expect(() => validateUrl(new URL('http://example.com:80'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('https://example.com:443'), allowedHosts)).not.toThrow();
});

it('should pass for explicitly allowed hostname and port', () => {
const allowedHosts = new Set(['example.com:8080']);
expect(() => validateUrl(new URL('http://example.com:8080'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('http://example.com:9090'), allowedHosts)).toThrowError(
/URL with host "example.com:9090" is not allowed/,
);
});

it('should pass for explicitly allowed wildcard hostname and port', () => {
const allowedHosts = new Set(['*.google.com:8443']);
expect(() => validateUrl(new URL('https://foo.google.com:8443'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('https://foo.google.com:9443'), allowedHosts)).toThrowError(
/URL with host "foo.google.com:9443" is not allowed/,
);
});
});

describe('validateRequest', () => {
Expand All @@ -97,10 +124,37 @@ describe('Validation Utils', () => {
const req = new Request('http://evil.com');

expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
/URL with hostname "evil.com" is not allowed/,
/URL with host "evil.com" is not allowed/,
);
});

it('should throw if URL port is not explicitly allowed', () => {
const req = new Request('http://example.com:8080');

expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
/URL with host "example.com:8080" is not allowed/,
);
});

it('should throw if x-forwarded-host uses an arbitrary port on an allowed hostname', () => {
const req = new Request('http://example.com:8080', {
headers: { 'x-forwarded-host': 'example.com:8080' },
});

expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
'Header "x-forwarded-host" with value "example.com:8080" is not allowed.',
);
});

it('should pass if x-forwarded-host uses an explicitly allowed port', () => {
const allowedHosts = new Set(['example.com:8080']);
const req = new Request('http://example.com:8080', {
headers: { 'x-forwarded-host': 'example.com:8080' },
});

expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
});

it('should throw if x-forwarded-port is invalid', () => {
const req = new Request('http://example.com', {
headers: { 'x-forwarded-port': 'abc' },
Expand Down
Loading