Skip to content

Commit bdc4f23

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 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 (`//`). Closes #32501
1 parent 2fd3b7c commit bdc4f23

File tree

4 files changed

+45
-9
lines changed

4 files changed

+45
-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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,4 +205,9 @@ function validateHeaders(request: Request, allowedHosts: ReadonlySet<string>): v
205205
if (xForwardedProto && !VALID_PROTO_REGEX.test(xForwardedProto)) {
206206
throw new Error('Header "x-forwarded-proto" must be either "http" or "https".');
207207
}
208+
209+
const xForwardedPrefix = getFirstHeaderValue(headers.get('x-forwarded-prefix'));
210+
if (xForwardedPrefix?.startsWith('//')) {
211+
throw new Error('Header "x-forwarded-prefix" must not start with multiple "/".');
212+
}
208213
}

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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,19 @@ describe('Validation Utils', () => {
124124
'Header "x-forwarded-host" contains path separators which is not allowed.',
125125
);
126126
});
127+
128+
it('should throw error if x-forwarded-prefix starts with multiple slashes', () => {
129+
const request = new Request('https://example.com', {
130+
headers: {
131+
'host': 'example.com',
132+
'x-forwarded-prefix': '//evil',
133+
},
134+
});
135+
136+
expect(() => validateRequest(request, allowedHosts)).toThrowError(
137+
'Header "x-forwarded-prefix" must not start with multiple "/".',
138+
);
139+
});
127140
});
128141

129142
describe('cloneRequestWithPatchedHeaders', () => {

0 commit comments

Comments
 (0)