Skip to content

Commit 8695d60

Browse files
committed
fix(@angular/ssr): prevent open redirect via X-Forwarded-Prefix header
This change addresses a security vulnerability where `joinUrlParts()` in `packages/angular/ssr/src/utils/url.ts` only stripped one leading slash from URL parts. When the `X-Forwarded-Prefix` header contains multiple leading slashes (e.g., `///evil.com`), the function previously produced a protocol-relative URL (e.g., `//evil.com/home`). If the application issues a redirect (e.g., via a generic redirect route), the browser interprets this 'Location' header as an external redirect to `https://evil.com/home`. This vulnerability poses a significant risk as open redirects can be used in phishing attacks. Additionally, since the redirect response may lack `Cache-Control` headers, intermediate CDNs could cache the poisoned redirect, serving it to other users. This commit fixes the issue by: 1. Updating `joinUrlParts` to internally strip *all* leading and trailing slashes from URL segments, preventing the formation of protocol-relative URLs from malicious input. 2. Adding strict validation for the `X-Forwarded-Prefix` header to immediately reject requests with values starting with multiple slashes (`//`) or backslashes (`\\`). Closes #32501
1 parent e4d445e commit 8695d60

File tree

4 files changed

+106
-9
lines changed

4 files changed

+106
-9
lines changed

packages/angular/ssr/src/utils/url.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,26 +95,32 @@ export function addTrailingSlash(url: string): string {
9595
* ```
9696
*/
9797
export function joinUrlParts(...parts: string[]): string {
98-
const normalizeParts: string[] = [];
98+
const normalizedParts: string[] = [];
99+
99100
for (const part of parts) {
100101
if (part === '') {
101102
// Skip any empty parts
102103
continue;
103104
}
104105

105-
let normalizedPart = part;
106-
if (part[0] === '/') {
107-
normalizedPart = normalizedPart.slice(1);
106+
let start = 0;
107+
let end = part.length;
108+
109+
// Use "Pointers" to avoid intermediate slices
110+
while (start < end && part[start] === '/') {
111+
start++;
108112
}
109-
if (part.at(-1) === '/') {
110-
normalizedPart = normalizedPart.slice(0, -1);
113+
114+
while (end > start && part[end - 1] === '/') {
115+
end--;
111116
}
112-
if (normalizedPart !== '') {
113-
normalizeParts.push(normalizedPart);
117+
118+
if (start < end) {
119+
normalizedParts.push(part.slice(start, end));
114120
}
115121
}
116122

117-
return addLeadingSlash(normalizeParts.join('/'));
123+
return addLeadingSlash(normalizedParts.join('/'));
118124
}
119125

120126
/**

packages/angular/ssr/src/utils/validation.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ const VALID_PROTO_REGEX = /^https?$/i;
2626
*/
2727
const VALID_HOST_REGEX = /^[a-z0-9.:-]+$/i;
2828

29+
/**
30+
* Regular expression to validate that the prefix is valid.
31+
*/
32+
const INVALID_PREFIX_REGEX = /^[/\\]{2}|(?:^|[/\\])\.\.?(?:[/\\]|$)/;
33+
2934
/**
3035
* Extracts the first value from a multi-value header string.
3136
*
@@ -253,4 +258,11 @@ function validateHeaders(request: Request): void {
253258
if (xForwardedProto && !VALID_PROTO_REGEX.test(xForwardedProto)) {
254259
throw new Error('Header "x-forwarded-proto" must be either "http" or "https".');
255260
}
261+
262+
const xForwardedPrefix = getFirstHeaderValue(headers.get('x-forwarded-prefix'));
263+
if (xForwardedPrefix && INVALID_PREFIX_REGEX.test(xForwardedPrefix)) {
264+
throw new Error(
265+
'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
266+
);
267+
}
256268
}

packages/angular/ssr/test/utils/url_spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,18 @@ describe('URL Utils', () => {
100100
it('should handle an all-empty URL parts', () => {
101101
expect(joinUrlParts('', '')).toBe('/');
102102
});
103+
104+
it('should normalize parts with multiple leading and trailing slashes', () => {
105+
expect(joinUrlParts('//path//', '///to///', '//resource//')).toBe('/path/to/resource');
106+
});
107+
108+
it('should handle a single part', () => {
109+
expect(joinUrlParts('path')).toBe('/path');
110+
});
111+
112+
it('should handle parts containing only slashes', () => {
113+
expect(joinUrlParts('//', '///')).toBe('/');
114+
});
103115
});
104116

105117
describe('stripIndexHtmlFromURL', () => {

packages/angular/ssr/test/utils/validation_spec.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,73 @@ describe('Validation Utils', () => {
124124
'Header "x-forwarded-host" contains characters that are not allowed.',
125125
);
126126
});
127+
128+
it('should throw error if x-forwarded-prefix starts with multiple slashes or backslashes', () => {
129+
const inputs = ['//evil', '\\\\evil', '/\\evil', '\\/evil'];
130+
131+
for (const prefix of inputs) {
132+
const request = new Request('https://example.com', {
133+
headers: {
134+
'x-forwarded-prefix': prefix,
135+
},
136+
});
137+
138+
expect(() => validateRequest(request, allowedHosts))
139+
.withContext(`Prefix: "${prefix}"`)
140+
.toThrowError(
141+
'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
142+
);
143+
}
144+
});
145+
146+
it('should throw error if x-forwarded-prefix contains dot segments', () => {
147+
const inputs = [
148+
'/./',
149+
'/../',
150+
'/foo/./bar',
151+
'/foo/../bar',
152+
'/.',
153+
'/..',
154+
'./',
155+
'../',
156+
'.\\',
157+
'..\\',
158+
'/foo/.\\bar',
159+
'/foo/..\\bar',
160+
'.',
161+
'..',
162+
];
163+
164+
for (const prefix of inputs) {
165+
const request = new Request('https://example.com', {
166+
headers: {
167+
'x-forwarded-prefix': prefix,
168+
},
169+
});
170+
171+
expect(() => validateRequest(request, allowedHosts))
172+
.withContext(`Prefix: "${prefix}"`)
173+
.toThrowError(
174+
'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
175+
);
176+
}
177+
});
178+
179+
it('should validate x-forwarded-prefix with valid dot usage', () => {
180+
const inputs = ['/foo.bar', '/foo.bar/baz', '/v1.2', '/.well-known'];
181+
182+
for (const prefix of inputs) {
183+
const request = new Request('https://example.com', {
184+
headers: {
185+
'x-forwarded-prefix': prefix,
186+
},
187+
});
188+
189+
expect(() => validateRequest(request, allowedHosts))
190+
.withContext(`Prefix: "${prefix}"`)
191+
.not.toThrow();
192+
}
193+
});
127194
});
128195

129196
describe('cloneRequestAndPatchHeaders', () => {

0 commit comments

Comments
 (0)