mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
fix(platform-server): throw on suspicious URLs and restrict protocol-relative URLs
Currently, the platform-server attempts to neutralize URL hijacking and SSRF bypasses by collapsing multiple leading slashes in relative paths. However, sophisticated bypasses using obfuscated protocols (e.g., carriage returns or newlines) or relative-like backslash paths can still lead to unexpected origin takeovers. This commit improves security by doing the following: - Rejects protocol-relative URLs by throwing an error if they are not explicitly permitted via `allowProtocolRelative`. - Strictly validates resolved URLs against the provided origin using `isSafeOriginChange`. If a URL unexpectedly shifts origins, an error is thrown. - Permits origin changes only when standard absolute http/https protocols are explicitly declared in the input URL.
This commit is contained in:
committed by
Pawel Kozlowski
parent
085e66ef03
commit
0b19c92d44
@@ -6,6 +6,11 @@
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
/**
|
||||
* Matches http: or https:
|
||||
*/
|
||||
const HTTP_OR_HTTPS_PROTOCOL_REGEX = /^https?:/i;
|
||||
|
||||
/**
|
||||
* Options for {@link resolveUrl}.
|
||||
*/
|
||||
@@ -36,17 +41,28 @@ export function resolveUrl(
|
||||
origin?: string | URL,
|
||||
options: ResolveUrlOptions = {},
|
||||
): URL | null {
|
||||
const originUrl = typeof origin === 'string' ? new URL('/', origin) : origin;
|
||||
|
||||
if (!urlStr) {
|
||||
return origin !== undefined ? new URL('/', origin) : null;
|
||||
return originUrl || null;
|
||||
}
|
||||
|
||||
urlStr = urlStr.trim();
|
||||
|
||||
// Fast-path: if the URL is a valid, standard absolute URL, parse and return it immediately.
|
||||
let resolved: URL | undefined;
|
||||
try {
|
||||
return new URL(urlStr);
|
||||
resolved = new URL(urlStr);
|
||||
} catch {}
|
||||
|
||||
if (resolved) {
|
||||
if (originUrl && !isSafeOriginChange(resolved, originUrl, urlStr)) {
|
||||
throwSuspiciousUrlError(urlStr);
|
||||
}
|
||||
|
||||
return resolved;
|
||||
}
|
||||
|
||||
// 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
|
||||
@@ -56,7 +72,7 @@ export function resolveUrl(
|
||||
throw new Error(`Invalid URL: ${urlStr}`);
|
||||
}
|
||||
|
||||
if (origin === undefined) {
|
||||
if (!originUrl) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -64,35 +80,40 @@ export function resolveUrl(
|
||||
|
||||
// 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.
|
||||
const isProtocolRelative =
|
||||
allowProtocolRelative &&
|
||||
urlStr[0] === '/' &&
|
||||
urlStr[1] === '/' &&
|
||||
urlStr.length > 2 &&
|
||||
urlStr[2] !== '/' &&
|
||||
urlStr[2] !== '\\';
|
||||
if (urlStr.startsWith('//')) {
|
||||
if (!allowProtocolRelative) {
|
||||
throw new Error(`Protocol relative URLs are not allowed in this context. URL: ${urlStr}`);
|
||||
}
|
||||
|
||||
if (isProtocolRelative) {
|
||||
return new URL(urlStr, origin);
|
||||
}
|
||||
|
||||
// Safe relative path preservation: if a relative path has no leading forward or backward slashes,
|
||||
// we do not prepend any slash so the native URL constructor can resolve it correctly relative
|
||||
// to trailing-slash sub-paths (e.g., 'testing' against 'http://localhost/foo/' -> 'http://localhost/foo/testing').
|
||||
const startsWithSlash = urlStr[0] === '/' || urlStr[0] === '\\';
|
||||
if (!startsWithSlash) {
|
||||
return new URL(urlStr, origin);
|
||||
resolved = new URL(urlStr, origin);
|
||||
|
||||
if (!isSafeOriginChange(resolved, originUrl, urlStr)) {
|
||||
throwSuspiciousUrlError(urlStr);
|
||||
}
|
||||
|
||||
// For other relative inputs starting with slashes, we collapse all consecutive leading forward/backward
|
||||
// slashes to a single forward slash. This guarantees consistent same-origin path representation and
|
||||
// blocks any hostname hijack or takeover attempts.
|
||||
let startIdx = 0;
|
||||
while (startIdx < urlStr.length && (urlStr[startIdx] === '/' || urlStr[startIdx] === '\\')) {
|
||||
startIdx++;
|
||||
}
|
||||
const pathWithoutLeadingSlashes = urlStr.slice(startIdx);
|
||||
const normalizedPath = '/' + pathWithoutLeadingSlashes;
|
||||
|
||||
return new URL(normalizedPath, origin);
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -1456,7 +1456,7 @@ class HiddenModule {}
|
||||
});
|
||||
});
|
||||
|
||||
it('prevents SSRF bypasses via backslash URLs in HttpClient', async () => {
|
||||
it('prevents SSRF bypasses via backslash URLs in HttpClient by throwing a suspicious origin error', async () => {
|
||||
const platform = platformServer([
|
||||
{
|
||||
provide: INITIAL_CONFIG,
|
||||
@@ -1467,11 +1467,16 @@ class HiddenModule {}
|
||||
const mock = ref.injector.get(HttpTestingController);
|
||||
const http = ref.injector.get(HttpClient);
|
||||
ref.injector.get(NgZone).run(() => {
|
||||
http.get('/\\evil.com/api').subscribe();
|
||||
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.`,
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
// To prevent SSRF, we ensures it's forced as a relative path and backslashes
|
||||
// inside path-relative segments are normalized via URL constructor, generating a safe URL.
|
||||
mock.expectOne('http://localhost:4000/evil.com/api').flush('safe');
|
||||
mock.verify();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1579,25 +1584,50 @@ class HiddenModule {}
|
||||
});
|
||||
});
|
||||
|
||||
it('should treat backslash bypass SSRF attempts in relative requests strictly as pathnames', async () => {
|
||||
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',
|
||||
'//\\attacker.com',
|
||||
' /\\attacker.com',
|
||||
'\r\n/\\attacker.com',
|
||||
];
|
||||
|
||||
ref.injector.get(NgZone).run(() => {
|
||||
for (const badUrl of badUrls) {
|
||||
http.get(badUrl).subscribe((body) => {
|
||||
expect(body).toEqual('success!');
|
||||
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
|
||||
.expectOne('http://localhost:4000/attacker.com', `URL: ${badUrl}`)
|
||||
.flush('success!');
|
||||
}
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -135,94 +135,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: '<html><head></head><body></body></html>',
|
||||
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: '<html><head></head><body></body></html>',
|
||||
url,
|
||||
},
|
||||
it('should throw on protocol-relative URLs in INITIAL_CONFIG', async () => {
|
||||
const platform = platformServer([
|
||||
{
|
||||
provide: INITIAL_CONFIG,
|
||||
useValue: {
|
||||
document: '<html><head></head><body></body></html>',
|
||||
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();
|
||||
});
|
||||
});
|
||||
})();
|
||||
|
||||
@@ -17,12 +17,12 @@ describe('resolveUrl', () => {
|
||||
expect(url.hash).toBe('#hash');
|
||||
});
|
||||
|
||||
it('should neutralize backslash-prefixed hijack attempts by forcing them same-origin', () => {
|
||||
it('should throw on backslash-prefixed hijack attempts', () => {
|
||||
const urls = ['/\\attacker.com/deep/path', '\\\\attacker.com/deep/path'];
|
||||
for (const url of urls) {
|
||||
const parsed = resolveUrl(url, 'http://test.com');
|
||||
expect(parsed.origin).toBe('http://test.com');
|
||||
expect(parsed.pathname).toBe('/attacker.com/deep/path');
|
||||
expect(() => resolveUrl(url, 'http://test.com')).toThrowError(
|
||||
`URL ${url} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -47,6 +47,13 @@ describe('resolveUrl', () => {
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('should throw on obfuscated protocols attempting to change origin', () => {
|
||||
const url = 'ht\ntp://evil.com/path';
|
||||
expect(() => resolveUrl(url, 'http://test.com')).toThrowError(
|
||||
`URL ${url} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('without origin', () => {
|
||||
|
||||
Reference in New Issue
Block a user