Skip to content
Merged
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: 3 additions & 1 deletion packages/angular/build/src/builders/dev-server/vite/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ export async function* serveWithVite(
// Angular SSR supports `*.`.
const allowedHosts = Array.isArray(serverOptions.allowedHosts)
? serverOptions.allowedHosts.map((host) => (host[0] === '.' ? '*' + host : host))
: [];
: serverOptions.allowedHosts === true
? ['*']
: [];

// Always allow the dev server host
allowedHosts.push(serverOptions.host);
Expand Down
17 changes: 16 additions & 1 deletion packages/angular/ssr/src/app-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,22 @@ export class AngularAppEngine {
* @param options Options for the Angular server application engine.
*/
constructor(options?: AngularAppEngineOptions) {
this.allowedHosts = new Set([...(options?.allowedHosts ?? []), ...this.manifest.allowedHosts]);
this.allowedHosts = this.getAllowedHosts(options);
}

private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {
const allowedHosts = new Set([...(options?.allowedHosts ?? []), ...this.manifest.allowedHosts]);

if (allowedHosts.has('*')) {
// eslint-disable-next-line no-console
console.warn(
'Allowing all hosts via "*" is a security risk. This configuration should only be used when ' +
'validation for "Host" and "X-Forwarded-Host" headers is performed in another layer, such as a load balancer or reverse proxy. ' +
'For more information see: https://angular.dev/best-practices/security#preventing-server-side-request-forgery-ssrf',
);
}

return allowedHosts;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/ssr/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ function verifyHostAllowed(
* @returns `true` if the hostname is allowed, `false` otherwise.
*/
function isHostAllowed(hostname: string, allowedHosts: ReadonlySet<string>): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Do we care about the case of allowedHosts = new Set(['*', 'example.test'])? Should we warn / error for extra hosts which will no-op or is it fine to just silently ignore this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's fine to ignore this case and just warn about *

if (allowedHosts.has(hostname)) {
if (allowedHosts.has('*') || allowedHosts.has(hostname)) {
return true;
}

Expand Down
16 changes: 16 additions & 0 deletions packages/angular/ssr/test/utils/validation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ describe('Validation Utils', () => {
/URL with hostname "google.com" is not allowed/,
);
});

it('should pass for all hostnames when "*" is used', () => {
const allowedHosts = new Set(['*']);
expect(() => validateUrl(new URL('http://example.com'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('http://google.com'), allowedHosts)).not.toThrow();
expect(() => validateUrl(new URL('http://evil.com'), allowedHosts)).not.toThrow();
});
});

describe('validateRequest', () => {
Expand Down Expand Up @@ -242,6 +249,15 @@ describe('Validation Utils', () => {
expect(secured.headers.get('host')).toBe('example.com');
});

it('should allow any host header when "*" is used', () => {
const allowedHosts = new Set(['*']);
const req = new Request('http://example.com', {
headers: { 'host': 'evil.com' },
});
const { request: secured } = cloneRequestAndPatchHeaders(req, allowedHosts);
expect(secured.headers.get('host')).toBe('evil.com');
});

it('should validate x-forwarded-host header', async () => {
const req = new Request('http://example.com', {
headers: { 'x-forwarded-host': 'evil.com' },
Expand Down
Loading