Sitelet https://github.com/angular/angular/commit/6ca433e56bcf74fdb6ad01d3afdf59628fba69b6
Skip to content

Commit 6ca433e

Browse files
alan-agius4pkozlowski-opensource
alan-agius4
authored and
pkozlowski-opensource
committed
fix(platform-server): throw on suspicious URLs and restrict protocol-relative URLs
Backports the security fixes from: - #68973 - #69018
1 parent c99d9f0 commit 6ca433e

5 files changed

Lines changed: 208 additions & 101 deletions

File tree

‎packages/platform-server/src/http.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
import {inject, Injectable, Provider} from '@angular/core';
1717
import {Observable} from 'rxjs';
1818

19+
import {parseUrl} from './url';
20+
1921
@Injectable()
2022
export class ServerXhr implements XhrFactory {
2123
private xhrImpl: typeof import('xhr2') | undefined;
@@ -41,10 +43,21 @@ export class ServerXhr implements XhrFactory {
4143
}
4244
}
4345

46+
/**
47+
* Regex to match a URL schema.
48+
*/
49+
const URL_SCHEMA_REGEXP = /^(?:[a-zA-Z][a-zA-Z0-9+\-.]*:)/;
50+
4451
function relativeUrlsTransformerInterceptorFn(
4552
request: HttpRequest<unknown>,
4653
next: HttpHandlerFn,
4754
): Observable<HttpEvent<unknown>> {
55+
const trimmedUrl = request.url.trim();
56+
if (URL_SCHEMA_REGEXP.test(trimmedUrl)) {
57+
// URLs with a schema should be left unchanged.
58+
return next(request);
59+
}
60+
4861
const platformLocation = inject(PlatformLocation);
4962
const {href, protocol, hostname, port} = platformLocation;
5063
if (!protocol.startsWith('http')) {
@@ -58,9 +71,11 @@ function relativeUrlsTransformerInterceptorFn(
5871

5972
const baseHref = platformLocation.getBaseHrefFromDOM() || href;
6073
const baseUrl = new URL(baseHref, urlPrefix);
61-
const newUrl = new URL(request.url, baseUrl).toString();
74+
const parsedUrl = parseUrl(request.url, baseUrl, {
75+
allowProtocolRelative: true,
76+
});
6277

63-
return next(request.clone({url: newUrl}));
78+
return next(request.clone({url: parsedUrl.toString()}));
6479
}
6580

6681
export const SERVER_HTTP_PROVIDERS: Provider[] = [

‎packages/platform-server/src/url.ts‎

Lines changed: 86 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,20 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
const LEADING_SLASHES_REGEX = /^[/\\]+/;
10-
const MALFORMED_ABSOLUTE_URL_REGEX = /^[a-zA-Z][a-zA-Z0-9+.-]*:(\/\/|\\\\)/;
9+
/**
10+
* Matches http: or https:
11+
*/
12+
const HTTP_OR_HTTPS_PROTOCOL_REGEX = /^https?:/i;
13+
14+
/**
15+
* Options for {@link parseUrl}.
16+
*/
17+
export interface ParseUrlOptions {
18+
/**
19+
* Allow protocol-relative URLs (e.g. `//example.com`).
20+
*/
21+
allowProtocolRelative?: boolean;
22+
}
1123

1224
/**
1325
* 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+.-]*:(\/\/|\\\\)/;
1628
* If an origin is provided, relative URLs and protocol-relative URLs are normalized and resolved against it.
1729
*/
1830
export function parseUrl(urlStr: string | undefined): URL | null;
19-
export function parseUrl(urlStr: string | undefined, origin: string): URL;
20-
export function parseUrl(urlStr: string | undefined, origin?: string): URL | null {
31+
export function parseUrl(
32+
urlStr: string | undefined,
33+
origin: string | URL,
34+
options?: ParseUrlOptions,
35+
): URL;
36+
export function parseUrl(
37+
urlStr: string | undefined,
38+
origin?: string | URL,
39+
options: ParseUrlOptions = {},
40+
): URL | null {
41+
const originUrl = typeof origin === 'string' ? new URL('/', origin) : origin;
42+
2143
if (!urlStr) {
22-
return origin !== undefined ? new URL('/', origin) : null;
44+
return originUrl || null;
2345
}
2446

25-
if (URL.canParse(urlStr)) {
26-
return new URL(urlStr);
47+
urlStr = urlStr.trim();
48+
49+
// Fast-path: if the URL is a valid, standard absolute URL, parse and return it immediately.
50+
let resolved: URL | undefined;
51+
try {
52+
resolved = new URL(urlStr);
53+
} catch {}
54+
55+
if (resolved) {
56+
if (originUrl && !isSafeOriginChange(resolved, originUrl, urlStr)) {
57+
throwSuspiciousUrlError(urlStr);
58+
}
59+
60+
return resolved;
2761
}
2862

29-
if (MALFORMED_ABSOLUTE_URL_REGEX.test(urlStr)) {
63+
// We identify and throw on malformed absolute URLs (like double port).
64+
// Per the WHATWG URL standard, parsing an input starting with a scheme (like 'http:') against
65+
// a standard base (like 'http://fake') ignores the base argument and parses strictly as an
66+
// absolute URL. Since it is malformed, the native URL constructor will throw a validation
67+
// error. Standard relative/protocol-relative paths parse successfully, allowing the flow to continue.
68+
if (!URL.canParse(urlStr, 'http://fake')) {
3069
throw new Error(`Invalid URL: ${urlStr}`);
3170
}
3271

33-
if (origin === undefined) {
72+
if (!originUrl) {
3473
return null;
3574
}
3675

37-
// Normalizes request path parsing by collapsing multiple consecutive leading slashes
38-
// and backslashes (e.g. // or /\) down to a single forward slash. This ensures consistent
39-
// resolution of relative path segments and prevents unexpected absolute path overrides
40-
// during URL parsing.
41-
let normalizedPath = urlStr.replace(LEADING_SLASHES_REGEX, '/');
42-
if (normalizedPath[0] !== '/') {
43-
normalizedPath = `/${normalizedPath}`;
76+
const {allowProtocolRelative = false} = options;
77+
78+
// Check if we have a legitimate protocol-relative URL (starts with '//' and not a duplicate/backslash bypass)
79+
// and we are configured to allow and preserve standard cross-origin protocol-relative requests.
80+
if (urlStr.startsWith('//')) {
81+
if (!allowProtocolRelative) {
82+
throw new Error(`Protocol relative URLs are not allowed in this context. URL: ${urlStr}`);
83+
}
84+
85+
return new URL(urlStr, origin);
86+
}
87+
88+
resolved = new URL(urlStr, origin);
89+
90+
if (!isSafeOriginChange(resolved, originUrl, urlStr)) {
91+
throwSuspiciousUrlError(urlStr);
4492
}
4593

46-
return new URL(normalizedPath, origin);
94+
return resolved;
95+
}
96+
97+
/**
98+
* Throws a suspicious URL error indicating a security bypass attempt.
99+
*/
100+
function throwSuspiciousUrlError(urlStr: string): never {
101+
throw new Error(
102+
`URL ${urlStr} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
103+
);
104+
}
105+
106+
/**
107+
* Checks if the origin has changed in a safe way.
108+
*
109+
* @param resolved The resolved URL.
110+
* @param origin The origin URL.
111+
* @param urlStr The URL string.
112+
* @returns True if the origin has changed in a safe way, false otherwise.
113+
*/
114+
function isSafeOriginChange(resolved: URL, origin: URL, urlStr: string): boolean {
115+
return origin.origin === resolved.origin || HTTP_OR_HTTPS_PROTOCOL_REGEX.test(urlStr);
47116
}

‎packages/platform-server/test/integration_spec.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,68 @@ class HiddenModule {}
15371537
mock.expectOne('http://localhost/testing').flush('success!');
15381538
});
15391539
});
1540+
1541+
it('prevents SSRF bypasses via backslash URLs in HttpClient by throwing a suspicious origin error', async () => {
1542+
ref.injector.get(NgZone).run(() => {
1543+
http.get('/\\evil.com/api').subscribe({
1544+
next: () => fail('Expected request to fail, but it succeeded.'),
1545+
error: (err) => {
1546+
expect(err.message).toBe(
1547+
`URL /\\evil.com/api changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
1548+
);
1549+
},
1550+
});
1551+
1552+
mock.verify();
1553+
});
1554+
});
1555+
1556+
it('should reject backslash bypass SSRF attempts in relative requests and throw a suspicious origin error', async () => {
1557+
const badUrls = [
1558+
'/\\attacker.com',
1559+
'\\\\attacker.com',
1560+
' /\\attacker.com',
1561+
'\r\n/\\attacker.com',
1562+
];
1563+
1564+
ref.injector.get(NgZone).run(() => {
1565+
for (const badUrl of badUrls) {
1566+
http.get(badUrl).subscribe({
1567+
next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`),
1568+
error: (err) => {
1569+
expect(err.message).toBe(
1570+
`URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
1571+
);
1572+
},
1573+
});
1574+
}
1575+
1576+
mock.verify();
1577+
});
1578+
});
1579+
1580+
it('should reject obfuscated protocal SSRF attempts in relative requests and throw a suspicious origin error', async () => {
1581+
const badUrls = [
1582+
'htt\rps://evil.com/path',
1583+
' htt\rps://evil.com/path',
1584+
'\r\nhtt\rps://evil.com/path',
1585+
];
1586+
1587+
ref.injector.get(NgZone).run(() => {
1588+
for (const badUrl of badUrls) {
1589+
http.get(badUrl).subscribe({
1590+
next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`),
1591+
error: (err) => {
1592+
expect(err.message).toBe(
1593+
`URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
1594+
);
1595+
},
1596+
});
1597+
}
1598+
1599+
mock.verify();
1600+
});
1601+
});
15401602
});
15411603
});
15421604
});

‎packages/platform-server/test/platform_location_spec.ts‎

Lines changed: 27 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {INITIAL_CONFIG, platformServer} from '@angular/platform-server';
3232
expect(location.pathname).toBe('/');
3333
platform.destroy();
3434
});
35+
3536
it('is configurable via INITIAL_CONFIG', async () => {
3637
const platform = platformServer([
3738
{
@@ -135,94 +136,38 @@ import {INITIAL_CONFIG, platformServer} from '@angular/platform-server';
135136
location.pushState(null, 'Test', '/foo#bar');
136137
});
137138

138-
it('neutralizes hostname hijack attempts', async () => {
139-
const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path'];
140-
141-
for (const url of urls) {
142-
const platform = platformServer([
143-
{
144-
provide: INITIAL_CONFIG,
145-
useValue: {
146-
document: '',
147-
// This should be treated as relative URL.
148-
// Example: `req.url: '//attacker.com/deep/path'` where request
149-
// to express server is 'http://localhost:4200//attacker.com/deep/path'.
150-
url,
151-
},
139+
it('should throw on hostname hijack attempts to prevent origin hijack', async () => {
140+
const platform = platformServer([
141+
{
142+
provide: INITIAL_CONFIG,
143+
useValue: {
144+
document: '<html><head></head><body></body></html>',
145+
url: '/\\attacker.com/deep/path',
152146
},
153-
]);
154-
155-
const location = platform.injector.get(PlatformLocation);
156-
platform.destroy();
147+
},
148+
]);
157149

158-
expect(location.hostname).withContext(`hostname for URL: "${url}"`).toBe('');
159-
expect(location.pathname)
160-
.withContext(`pathname for URL: "${url}"`)
161-
.toBe('/attacker.com/deep/path');
162-
}
150+
expect(() => platform.injector.get(DOCUMENT)).toThrowError(
151+
`URL /\\attacker.com/deep/path changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
152+
);
153+
platform.destroy();
163154
});
164155

165-
it('should set the proper document location when the URL has leading slashes to prevent origin hijack', async () => {
166-
const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path'];
167-
168-
for (const url of urls) {
169-
const platform = platformServer([
170-
{
171-
provide: INITIAL_CONFIG,
172-
useValue: {
173-
document: '<html><head></head><body></body></html>',
174-
url,
175-
},
156+
it('should throw on protocol-relative URLs in INITIAL_CONFIG', async () => {
157+
const platform = platformServer([
158+
{
159+
provide: INITIAL_CONFIG,
160+
useValue: {
161+
document: '<html><head></head><body></body></html>',
162+
url: '//attacker.com/deep/path',
176163
},
177-
]);
178-
179-
const doc = platform.injector.get(DOCUMENT);
180-
platform.destroy();
181-
182-
expect(doc.location.origin).not.toBe('http://attacker.com');
183-
expect(doc.location.pathname).toBe('/attacker.com/deep/path');
184-
}
185-
});
164+
},
165+
]);
186166

187-
it('should not expose protocol-relative URLs on the location to prevent open redirect and SSRF bypasses', async () => {
188-
const urls = ['/\\attacker.com/deep/path', '//attacker.com/deep/path'];
189-
const origins = [undefined, 'http://localhost:4200'];
190-
191-
for (const url of urls) {
192-
for (const origin of origins) {
193-
const providers: any[] = [
194-
{
195-
provide: INITIAL_CONFIG,
196-
useValue: {
197-
document: '',
198-
url,
199-
},
200-
},
201-
];
202-
203-
if (origin) {
204-
providers.push({
205-
provide: DOCUMENT,
206-
useValue: {
207-
location: {
208-
origin,
209-
},
210-
},
211-
});
212-
}
213-
214-
const platform = platformServer(providers);
215-
const location = platform.injector.get(PlatformLocation) as any;
216-
platform.destroy();
217-
218-
// A relative redirect URL starting with // or /\ is normalized by browsers to a protocol-relative URL.
219-
// The PlatformLocation.url property MUST NOT expose these unsafe patterns.
220-
const isVulnerable = location.url.startsWith('//') || location.url.startsWith('/\\');
221-
expect(isVulnerable)
222-
.withContext(`URL: "${url}", origin: "${origin}", location.url: "${location.url}"`)
223-
.toBeFalse();
224-
}
225-
}
167+
expect(() => platform.injector.get(DOCUMENT)).toThrowError(
168+
`Protocol relative URLs are not allowed in this context. URL: //attacker.com/deep/path`,
169+
);
170+
platform.destroy();
226171
});
227172
});
228173
})();

0 commit comments

Comments
 (0)