diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts index d0c2f88060b..d977b343427 100644 --- a/packages/platform-server/src/http.ts +++ b/packages/platform-server/src/http.ts @@ -16,6 +16,8 @@ import { import {inject, Injectable, Provider} from '@angular/core'; import {Observable} from 'rxjs'; +import {parseUrl} from './url'; + @Injectable() export class ServerXhr implements XhrFactory { private xhrImpl: typeof import('xhr2') | undefined; @@ -41,10 +43,21 @@ export class ServerXhr implements XhrFactory { } } +/** + * Regex to match a URL schema. + */ +const URL_SCHEMA_REGEXP = /^(?:[a-zA-Z][a-zA-Z0-9+\-.]*:)/; + function relativeUrlsTransformerInterceptorFn( request: HttpRequest, next: HttpHandlerFn, ): Observable> { + const trimmedUrl = request.url.trim(); + if (URL_SCHEMA_REGEXP.test(trimmedUrl)) { + // URLs with a schema should be left unchanged. + return next(request); + } + const platformLocation = inject(PlatformLocation); const {href, protocol, hostname, port} = platformLocation; if (!protocol.startsWith('http')) { @@ -58,9 +71,11 @@ function relativeUrlsTransformerInterceptorFn( const baseHref = platformLocation.getBaseHrefFromDOM() || href; const baseUrl = new URL(baseHref, urlPrefix); - const newUrl = new URL(request.url, baseUrl).toString(); + const parsedUrl = parseUrl(request.url, baseUrl, { + allowProtocolRelative: true, + }); - return next(request.clone({url: newUrl})); + return next(request.clone({url: parsedUrl.toString()})); } export const SERVER_HTTP_PROVIDERS: Provider[] = [ diff --git a/packages/platform-server/src/url.ts b/packages/platform-server/src/url.ts index 735900eb9ce..8be88bdd800 100644 --- a/packages/platform-server/src/url.ts +++ b/packages/platform-server/src/url.ts @@ -6,8 +6,20 @@ * found in the LICENSE file at https://angular.dev/license */ -const LEADING_SLASHES_REGEX = /^[/\\]+/; -const MALFORMED_ABSOLUTE_URL_REGEX = /^[a-zA-Z][a-zA-Z0-9+.-]*:(\/\/|\\\\)/; +/** + * Matches http: or https: + */ +const HTTP_OR_HTTPS_PROTOCOL_REGEX = /^https?:/i; + +/** + * Options for {@link parseUrl}. + */ +export interface ParseUrlOptions { + /** + * Allow protocol-relative URLs (e.g. `//example.com`). + */ + allowProtocolRelative?: boolean; +} /** * Parses a URL string and returns a resolved WHATWG URL object. @@ -16,32 +28,89 @@ const MALFORMED_ABSOLUTE_URL_REGEX = /^[a-zA-Z][a-zA-Z0-9+.-]*:(\/\/|\\\\)/; * If an origin is provided, relative URLs and protocol-relative URLs are normalized and resolved against it. */ export function parseUrl(urlStr: string | undefined): URL | null; -export function parseUrl(urlStr: string | undefined, origin: string): URL; -export function parseUrl(urlStr: string | undefined, origin?: string): URL | null { +export function parseUrl( + urlStr: string | undefined, + origin: string | URL, + options?: ParseUrlOptions, +): URL; +export function parseUrl( + urlStr: string | undefined, + origin?: string | URL, + options: ParseUrlOptions = {}, +): URL | null { + const originUrl = typeof origin === 'string' ? new URL('/', origin) : origin; + if (!urlStr) { - return origin !== undefined ? new URL('/', origin) : null; + return originUrl || null; } - if (URL.canParse(urlStr)) { - return new URL(urlStr); + urlStr = urlStr.trim(); + + // Fast-path: if the URL is a valid, standard absolute URL, parse and return it immediately. + let resolved: URL | undefined; + try { + resolved = new URL(urlStr); + } catch {} + + if (resolved) { + if (originUrl && !isSafeOriginChange(resolved, originUrl, urlStr)) { + throwSuspiciousUrlError(urlStr); + } + + return resolved; } - if (MALFORMED_ABSOLUTE_URL_REGEX.test(urlStr)) { + // We identify and throw on malformed absolute URLs (like double port). + // Per the WHATWG URL standard, parsing an input starting with a scheme (like 'http:') against + // a standard base (like 'http://fake') ignores the base argument and parses strictly as an + // absolute URL. Since it is malformed, the native URL constructor will throw a validation + // error. Standard relative/protocol-relative paths parse successfully, allowing the flow to continue. + if (!URL.canParse(urlStr, 'http://fake')) { throw new Error(`Invalid URL: ${urlStr}`); } - if (origin === undefined) { + if (!originUrl) { return null; } - // Normalizes request path parsing by collapsing multiple consecutive leading slashes - // and backslashes (e.g. // or /\) down to a single forward slash. This ensures consistent - // resolution of relative path segments and prevents unexpected absolute path overrides - // during URL parsing. - let normalizedPath = urlStr.replace(LEADING_SLASHES_REGEX, '/'); - if (normalizedPath[0] !== '/') { - normalizedPath = `/${normalizedPath}`; + const {allowProtocolRelative = false} = options; + + // Check if we have a legitimate protocol-relative URL (starts with '//' and not a duplicate/backslash bypass) + // and we are configured to allow and preserve standard cross-origin protocol-relative requests. + if (urlStr.startsWith('//')) { + if (!allowProtocolRelative) { + throw new Error(`Protocol relative URLs are not allowed in this context. URL: ${urlStr}`); + } + + return new URL(urlStr, origin); } - return new URL(normalizedPath, origin); + resolved = new URL(urlStr, origin); + + if (!isSafeOriginChange(resolved, originUrl, urlStr)) { + throwSuspiciousUrlError(urlStr); + } + + return resolved; +} + +/** + * Throws a suspicious URL error indicating a security bypass attempt. + */ +function throwSuspiciousUrlError(urlStr: string): never { + throw new Error( + `URL ${urlStr} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); +} + +/** + * Checks if the origin has changed in a safe way. + * + * @param resolved The resolved URL. + * @param origin The origin URL. + * @param urlStr The URL string. + * @returns True if the origin has changed in a safe way, false otherwise. + */ +function isSafeOriginChange(resolved: URL, origin: URL, urlStr: string): boolean { + return origin.origin === resolved.origin || HTTP_OR_HTTPS_PROTOCOL_REGEX.test(urlStr); } diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 5dfe5044a31..922037c5b59 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -1537,6 +1537,68 @@ class HiddenModule {} mock.expectOne('http://localhost/testing').flush('success!'); }); }); + + it('prevents SSRF bypasses via backslash URLs in HttpClient by throwing a suspicious origin error', async () => { + ref.injector.get(NgZone).run(() => { + http.get('/\\evil.com/api').subscribe({ + next: () => fail('Expected request to fail, but it succeeded.'), + error: (err) => { + expect(err.message).toBe( + `URL /\\evil.com/api changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + }, + }); + + mock.verify(); + }); + }); + + it('should reject backslash bypass SSRF attempts in relative requests and throw a suspicious origin error', async () => { + const badUrls = [ + '/\\attacker.com', + '\\\\attacker.com', + ' /\\attacker.com', + '\r\n/\\attacker.com', + ]; + + ref.injector.get(NgZone).run(() => { + for (const badUrl of badUrls) { + http.get(badUrl).subscribe({ + next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`), + error: (err) => { + expect(err.message).toBe( + `URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + }, + }); + } + + mock.verify(); + }); + }); + + it('should reject obfuscated protocal SSRF attempts in relative requests and throw a suspicious origin error', async () => { + const badUrls = [ + 'htt\rps://evil.com/path', + ' htt\rps://evil.com/path', + '\r\nhtt\rps://evil.com/path', + ]; + + ref.injector.get(NgZone).run(() => { + for (const badUrl of badUrls) { + http.get(badUrl).subscribe({ + next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`), + error: (err) => { + expect(err.message).toBe( + `URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + }, + }); + } + + mock.verify(); + }); + }); }); }); }); diff --git a/packages/platform-server/test/platform_location_spec.ts b/packages/platform-server/test/platform_location_spec.ts index ec5a1f6e5ab..2f32de1a3c5 100644 --- a/packages/platform-server/test/platform_location_spec.ts +++ b/packages/platform-server/test/platform_location_spec.ts @@ -32,6 +32,7 @@ import {INITIAL_CONFIG, platformServer} from '@angular/platform-server'; expect(location.pathname).toBe('/'); platform.destroy(); }); + it('is configurable via INITIAL_CONFIG', async () => { const platform = platformServer([ { @@ -135,94 +136,38 @@ import {INITIAL_CONFIG, platformServer} from '@angular/platform-server'; location.pushState(null, 'Test', '/foo#bar'); }); - it('neutralizes hostname hijack attempts', async () => { - const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path']; - - for (const url of urls) { - const platform = platformServer([ - { - provide: INITIAL_CONFIG, - useValue: { - document: '', - // This should be treated as relative URL. - // Example: `req.url: '//attacker.com/deep/path'` where request - // to express server is 'http://localhost:4200//attacker.com/deep/path'. - url, - }, + it('should throw on hostname hijack attempts to prevent origin hijack', async () => { + const platform = platformServer([ + { + provide: INITIAL_CONFIG, + useValue: { + document: '', + url: '/\\attacker.com/deep/path', }, - ]); + }, + ]); - const location = platform.injector.get(PlatformLocation); - platform.destroy(); - - expect(location.hostname).withContext(`hostname for URL: "${url}"`).toBe(''); - expect(location.pathname) - .withContext(`pathname for URL: "${url}"`) - .toBe('/attacker.com/deep/path'); - } + expect(() => platform.injector.get(DOCUMENT)).toThrowError( + `URL /\\attacker.com/deep/path changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + platform.destroy(); }); - it('should set the proper document location when the URL has leading slashes to prevent origin hijack', async () => { - const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path']; - - for (const url of urls) { - const platform = platformServer([ - { - provide: INITIAL_CONFIG, - useValue: { - document: '', - url, - }, + it('should throw on protocol-relative URLs in INITIAL_CONFIG', async () => { + const platform = platformServer([ + { + provide: INITIAL_CONFIG, + useValue: { + document: '', + url: '//attacker.com/deep/path', }, - ]); + }, + ]); - const doc = platform.injector.get(DOCUMENT); - platform.destroy(); - - expect(doc.location.origin).not.toBe('http://attacker.com'); - expect(doc.location.pathname).toBe('/attacker.com/deep/path'); - } - }); - - it('should not expose protocol-relative URLs on the location to prevent open redirect and SSRF bypasses', async () => { - const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path']; - const origins = [undefined, 'http://localhost:4200']; - - for (const url of urls) { - for (const origin of origins) { - const providers: any[] = [ - { - provide: INITIAL_CONFIG, - useValue: { - document: '', - url, - }, - }, - ]; - - if (origin) { - providers.push({ - provide: DOCUMENT, - useValue: { - location: { - origin, - }, - }, - }); - } - - const platform = platformServer(providers); - const location = platform.injector.get(PlatformLocation) as any; - platform.destroy(); - - // A relative redirect URL starting with // or /\ is normalized by browsers to a protocol-relative URL. - // The PlatformLocation.url property MUST NOT expose these unsafe patterns. - const isVulnerable = location.url.startsWith('//') || location.url.startsWith('/\\'); - expect(isVulnerable) - .withContext(`URL: "${url}", origin: "${origin}", location.url: "${location.url}"`) - .toBeFalse(); - } - } + expect(() => platform.injector.get(DOCUMENT)).toThrowError( + `Protocol relative URLs are not allowed in this context. URL: //attacker.com/deep/path`, + ); + platform.destroy(); }); }); })(); diff --git a/packages/platform-server/test/url_spec.ts b/packages/platform-server/test/url_spec.ts index 95d9095bf3e..dbb5c38f93a 100644 --- a/packages/platform-server/test/url_spec.ts +++ b/packages/platform-server/test/url_spec.ts @@ -17,6 +17,15 @@ describe('parseUrl', () => { expect(url.hash).toBe('#hash'); }); + it('should throw on backslash-prefixed hijack attempts', () => { + const urls = ['/\\attacker.com/deep/path', '\\\\attacker.com/deep/path']; + for (const url of urls) { + expect(() => parseUrl(url, 'http://test.com')).toThrowError( + `URL ${url} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + } + }); + it('should resolve absolute URLs ignoring origin', () => { const url = parseUrl('http://other.com/deep/path', 'http://test.com'); expect(url.href).toBe('http://other.com/deep/path'); @@ -38,6 +47,13 @@ describe('parseUrl', () => { ); } }); + + it('should throw on obfuscated protocols attempting to change origin', () => { + const url = 'ht\ntp://evil.com/path'; + expect(() => parseUrl(url, 'http://test.com')).toThrowError( + `URL ${url} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`, + ); + }); }); describe('without origin', () => {