Skip to content

Commit cf9d7fa

Browse files
alan-agius4atscott
alan-agius4
authored and
atscott
committed
fix(platform-server): harden platform location origin validation during SSR (#69184)
Add allowOriginChange option to ResolveUrlOptions in resolveUrl to enforce same-origin validation on resolved URLs. When set to false, it prevents any cross-origin changes (including HTTP/HTTPS URLs), aligning the emulated server-side platform location environment with browser security behavior. Refactor ServerPlatformLocation.replaceState to use allowOriginChange: false instead of manual comparison, hardening state change validation against cross-origin URLs. Add unit tests in url_spec.ts and platform_location_spec.ts for the origin validation changes. PR Close #69184
1 parent abfb04a commit cf9d7fa

6 files changed

Lines changed: 112 additions & 14 deletions

File tree

‎packages/core/src/errors.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import {ERROR_DETAILS_PAGE_BASE_URL} from './error_details_base_url';
2525
* - animations: 3000-3999
2626
* - router: 4000-4999
2727
* - platform-browser: 5000-5500
28+
* - service-worker: 5600-5699
29+
* - platform-server: 5700-5800
2830
*/
2931
export const enum RuntimeErrorCode {
3032
// Change Detection Errors

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,18 @@ export class ServerPlatformLocation implements PlatformLocation {
3333
public readonly search: string = '';
3434
public readonly hash: string = '';
3535
private _hashUpdate = new Subject<LocationChangeEvent>();
36-
private _doc = inject(DOCUMENT);
36+
private readonly _doc = inject(DOCUMENT);
37+
private readonly origin = this._doc.location.origin;
3738

3839
constructor() {
3940
const config = inject(INITIAL_CONFIG, {optional: true});
4041
if (!config) {
4142
return;
4243
}
4344
if (config.url) {
44-
const {protocol, hostname, port, pathname, search, hash, href} = resolveUrl(
45+
const {protocol, hostname, port, pathname, search, hash, href, origin} = resolveUrl(
4546
config.url,
46-
this._doc.location.origin,
47+
this.origin,
4748
);
4849
this.protocol = protocol;
4950
this.hostname = hostname;
@@ -52,6 +53,7 @@ export class ServerPlatformLocation implements PlatformLocation {
5253
this.search = search;
5354
this.hash = hash;
5455
this.href = href;
56+
this.origin = origin;
5557
}
5658
}
5759

@@ -93,7 +95,9 @@ export class ServerPlatformLocation implements PlatformLocation {
9395

9496
replaceState(state: any, title: string, newUrl: string): void {
9597
const oldUrl = this.url;
96-
const {pathname, search, hash, href, protocol} = resolveUrl(newUrl, this._doc.location.origin);
98+
const {pathname, search, hash, href, protocol} = resolveUrl(newUrl, this.origin, {
99+
allowOriginChange: false,
100+
});
97101
const writableThis = this as Writable<this>;
98102
writableThis.pathname = pathname;
99103
writableThis.search = search;

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

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ const HTTP_OR_HTTPS_PROTOCOL_REGEX = /^https?:/i;
2121
export interface ResolveUrlOptions {
2222
/**
2323
* Allow protocol-relative URLs (e.g. `//example.com`).
24+
* @default false
2425
*/
2526
allowProtocolRelative?: boolean;
27+
/**
28+
* Allow origin changes.
29+
* @default true
30+
*/
31+
allowOriginChange?: boolean;
2632
}
2733

2834
/**
@@ -58,9 +64,10 @@ export function resolveUrl(
5864
try {
5965
resolved = new URL(urlStr);
6066
} catch {}
67+
const {allowProtocolRelative = false, allowOriginChange = true} = options;
6168

6269
if (resolved) {
63-
if (originUrl && !isSafeOriginChange(resolved, originUrl, urlStr)) {
70+
if (originUrl && !isSafeOriginChange(resolved, originUrl, urlStr, allowOriginChange)) {
6471
throwSuspiciousUrlError(urlStr);
6572
}
6673

@@ -75,23 +82,21 @@ export function resolveUrl(
7582
if (!URL.canParse(urlStr, 'http://fake')) {
7683
throw new RuntimeError(
7784
RuntimeErrorCode.INVALID_URL,
78-
ngDevMode ? `Invalid URL: ${urlStr}` : urlStr,
85+
typeof ngDevMode === 'undefined' || ngDevMode ? `Invalid URL: ${urlStr}` : urlStr,
7986
);
8087
}
8188

8289
if (!originUrl) {
8390
return null;
8491
}
8592

86-
const {allowProtocolRelative = false} = options;
87-
8893
// Check if we have a legitimate protocol-relative URL (starts with '//' and not a duplicate/backslash bypass)
8994
// and we are configured to allow and preserve standard cross-origin protocol-relative requests.
9095
if (urlStr.startsWith('//')) {
9196
if (!allowProtocolRelative) {
9297
throw new RuntimeError(
9398
RuntimeErrorCode.PROTOCOL_RELATIVE_URL_NOT_ALLOWED,
94-
ngDevMode
99+
typeof ngDevMode === 'undefined' || ngDevMode
95100
? `Protocol relative URLs are not allowed in this context. URL: ${urlStr}`
96101
: urlStr,
97102
);
@@ -102,7 +107,7 @@ export function resolveUrl(
102107

103108
resolved = new URL(urlStr, origin);
104109

105-
if (!isSafeOriginChange(resolved, originUrl, urlStr)) {
110+
if (!isSafeOriginChange(resolved, originUrl, urlStr, allowOriginChange)) {
106111
throwSuspiciousUrlError(urlStr);
107112
}
108113

@@ -115,7 +120,7 @@ export function resolveUrl(
115120
function throwSuspiciousUrlError(urlStr: string): never {
116121
throw new RuntimeError(
117122
RuntimeErrorCode.SUSPICIOUS_URL_CHANGE_ORIGIN,
118-
ngDevMode
123+
typeof ngDevMode === 'undefined' || ngDevMode
119124
? `URL ${urlStr} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`
120125
: urlStr,
121126
);
@@ -127,8 +132,22 @@ function throwSuspiciousUrlError(urlStr: string): never {
127132
* @param resolved The resolved URL.
128133
* @param origin The origin URL.
129134
* @param urlStr The URL string.
135+
* @param allowOriginChange Whether to allow origin changes.
130136
* @returns True if the origin has changed in a safe way, false otherwise.
131137
*/
132-
function isSafeOriginChange(resolved: URL, origin: URL, urlStr: string): boolean {
133-
return origin.origin === resolved.origin || HTTP_OR_HTTPS_PROTOCOL_REGEX.test(urlStr);
138+
function isSafeOriginChange(
139+
resolved: URL,
140+
origin: URL,
141+
urlStr: string,
142+
allowOriginChange: boolean,
143+
): boolean {
144+
if (origin.origin === resolved.origin) {
145+
return true;
146+
}
147+
148+
if (!allowOriginChange) {
149+
return false;
150+
}
151+
152+
return HTTP_OR_HTTPS_PROTOCOL_REGEX.test(urlStr);
134153
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ function validateAllowedHosts(url: string | undefined, allowedHosts: string[] |
388388
if (!isHostAllowed(hostname, allowedHostsSet)) {
389389
throw new RuntimeError(
390390
RuntimeErrorCode.HOST_NOT_ALLOWED,
391-
ngDevMode
391+
typeof ngDevMode === 'undefined' || ngDevMode
392392
? `Host ${url} is not allowed. You can configure \`allowedHosts\` option.`
393393
: url,
394394
);

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,5 +168,58 @@ import {INITIAL_CONFIG, platformServer} from '@angular/platform-server';
168168
);
169169
platform.destroy();
170170
});
171+
172+
it('should throw on replaceState with different origin', async () => {
173+
const platform = platformServer([
174+
{
175+
provide: INITIAL_CONFIG,
176+
useValue: {
177+
document: '<html><head></head><body></body></html>',
178+
url: 'http://test.com/deep/path',
179+
},
180+
},
181+
]);
182+
183+
const location = platform.injector.get(PlatformLocation);
184+
expect(() => location.replaceState(null, 'Title', 'http://attacker.com/foo')).toThrowError(
185+
`NG05703: URL http://attacker.com/foo changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
186+
);
187+
platform.destroy();
188+
});
189+
190+
it('should throw on pushState with different origin', async () => {
191+
const platform = platformServer([
192+
{
193+
provide: INITIAL_CONFIG,
194+
useValue: {
195+
document: '<html><head></head><body></body></html>',
196+
url: 'http://test.com/deep/path',
197+
},
198+
},
199+
]);
200+
201+
const location = platform.injector.get(PlatformLocation);
202+
expect(() => location.pushState(null, 'Title', 'http://attacker.com/foo')).toThrowError(
203+
`NG05703: URL http://attacker.com/foo changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
204+
);
205+
platform.destroy();
206+
});
207+
208+
it('should allow replaceState/pushState with same origin', async () => {
209+
const platform = platformServer([
210+
{
211+
provide: INITIAL_CONFIG,
212+
useValue: {
213+
document: '<html><head></head><body></body></html>',
214+
url: 'http://test.com/deep/path',
215+
},
216+
},
217+
]);
218+
219+
const location = platform.injector.get(PlatformLocation);
220+
expect(() => location.replaceState(null, 'Title', '/other-path')).not.toThrow();
221+
expect(() => location.pushState(null, 'Title', 'http://test.com/other-path')).not.toThrow();
222+
platform.destroy();
223+
});
171224
});
172225
})();

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@ describe('resolveUrl', () => {
3232
expect(url.origin).toBe('http://other.com');
3333
});
3434

35+
it('should throw when allowOriginChange is false and origin changes', () => {
36+
expect(() =>
37+
resolveUrl('http://other.com/deep/path', 'http://test.com', {allowOriginChange: false}),
38+
).toThrowError(
39+
`NG05703: URL http://other.com/deep/path changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
40+
);
41+
});
42+
43+
it('should resolve same origin when allowOriginChange is false', () => {
44+
const url = resolveUrl('http://test.com/other-path', 'http://test.com', {
45+
allowOriginChange: false,
46+
});
47+
expect(url.href).toBe('http://test.com/other-path');
48+
});
49+
50+
it('should resolve relative paths when allowOriginChange is false', () => {
51+
const url = resolveUrl('/other-path', 'http://test.com', {allowOriginChange: false});
52+
expect(url.href).toBe('http://test.com/other-path');
53+
});
54+
3555
it('should throw an error for malformed absolute URLs', () => {
3656
const malformedUrls = [
3757
'http://evil.com:80:80/path',

0 commit comments

Comments
 (0)