Skip to content

Commit c4f66cc

Browse files
committed
fix(@angular/build): escape prerender redirect URLs
Escape prerender redirect targets before embedding them in generated static redirect pages. This prevents attacker-controlled redirect URLs from breaking out of the meta refresh, anchor href, or fallback text contexts and injecting HTML that could lead to XSS.
1 parent 60481e9 commit c4f66cc

7 files changed

Lines changed: 318 additions & 8 deletions

File tree

packages/angular/build/src/utils/server-rendering/prerender.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
WritableSerializableRouteTreeNode,
2727
} from './models';
2828
import type { RenderWorkerData } from './render-worker';
29-
import { generateRedirectStaticPage } from './utils';
29+
import { generateRedirectStaticPage, validateStaticRedirectUrl } from './utils';
3030

3131
type PrerenderOptions = NormalizedApplicationBuildOptions['prerenderOptions'];
3232
type AppShellOptions = NormalizedApplicationBuildOptions['appShellOptions'];
@@ -131,6 +131,14 @@ export async function prerenderPages(
131131

132132
const serializableRouteTreeNodeForPrerender: WritableSerializableRouteTreeNode = [];
133133
for (const metadata of serializableRouteTreeNode) {
134+
if (outputMode === OutputMode.Static) {
135+
const invalidStaticRedirect = validateExtractedStaticRedirect(metadata);
136+
if (invalidStaticRedirect) {
137+
errors.push(invalidStaticRedirect);
138+
continue;
139+
}
140+
}
141+
134142
if (outputMode !== OutputMode.Static && metadata.redirectTo) {
135143
// Skip redirects if output mode is not static.
136144
continue;
@@ -290,6 +298,37 @@ async function renderPages(
290298
};
291299
}
292300

301+
function validateExtractedStaticRedirect({
302+
route,
303+
redirectTo,
304+
headers,
305+
}: SerializableRouteTreeNode[number]): string | undefined {
306+
if (redirectTo !== undefined) {
307+
const invalidRedirectReason = validateStaticRedirectUrl(redirectTo);
308+
if (invalidRedirectReason) {
309+
return (
310+
`Invalid 'redirectTo' for route '${stripLeadingSlash(route)}': ${invalidRedirectReason} ` +
311+
`Such values would be embedded verbatim in the generated static redirect page, ` +
312+
`which can lead to HTML injection. Percent-encode the value or sanitize the source.`
313+
);
314+
}
315+
}
316+
317+
const location = headers?.['Location'] ?? headers?.['location'];
318+
if (location !== undefined) {
319+
const invalidLocationReason = validateStaticRedirectUrl(location);
320+
if (invalidLocationReason) {
321+
return (
322+
`Invalid 'headers.Location' for route '${stripLeadingSlash(route)}': ${invalidLocationReason} ` +
323+
`Such values would be embedded verbatim in the generated static redirect page, ` +
324+
`which can lead to HTML injection. Percent-encode the value or sanitize the source.`
325+
);
326+
}
327+
}
328+
329+
return undefined;
330+
}
331+
293332
async function getAllRoutes(
294333
workspaceRoot: string,
295334
baseHref: string,

packages/angular/build/src/utils/server-rendering/utils.ts

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,90 @@ export function isSsrRequestHandler(
2222
return typeof value === 'function' && '__ng_request_handler__' in value;
2323
}
2424

25+
const htmlEscapeCharacters: Record<string, string> = {
26+
'&': '&amp;',
27+
'<': '&lt;',
28+
'>': '&gt;',
29+
'"': '&quot;',
30+
"'": '&#39;',
31+
};
32+
33+
function escapeHtml(text: string): string {
34+
return text.replace(/[&<>"']/g, (character) => htmlEscapeCharacters[character]);
35+
}
36+
37+
const unsafeStaticRedirectCharacters = /[\\<>"'`]/;
38+
const allowedStaticRedirectProtocols = new Set(['http:', 'https:']);
39+
40+
function hasUnsafeStaticRedirectCharacters(value: string): boolean {
41+
for (let index = 0; index < value.length; index++) {
42+
const characterCode = value.charCodeAt(index);
43+
if (characterCode <= 0x20 || characterCode === 0x7f) {
44+
return true;
45+
}
46+
}
47+
48+
return unsafeStaticRedirectCharacters.test(value);
49+
}
50+
51+
export function validateStaticRedirectUrl(url: string): string | undefined {
52+
if (hasUnsafeStaticRedirectCharacters(url)) {
53+
return (
54+
`the value '${url}' contains characters that are not allowed in a statically ` +
55+
`emitted URL (control characters, whitespace, backslash, or HTML-significant characters).`
56+
);
57+
}
58+
59+
if (url.startsWith('/')) {
60+
return url.startsWith('//')
61+
? 'protocol-relative URLs are not supported for statically emitted redirects. ' +
62+
'Only HTTP(S) URLs and same-origin absolute paths are supported.'
63+
: undefined;
64+
}
65+
66+
const schemeMatch = /^([a-zA-Z][a-zA-Z0-9+\-.]*):/.exec(url);
67+
if (!schemeMatch) {
68+
return (
69+
`the value '${url}' is not a valid absolute URL or same-origin absolute path. ` +
70+
`Only HTTP(S) URLs and same-origin absolute paths are supported.`
71+
);
72+
}
73+
74+
const protocol = `${schemeMatch[1].toLowerCase()}:`;
75+
if (!allowedStaticRedirectProtocols.has(protocol)) {
76+
return (
77+
`the protocol '${protocol}' is not allowed for a statically emitted redirect. ` +
78+
`Only HTTP(S) URLs and same-origin absolute paths are supported.`
79+
);
80+
}
81+
82+
return undefined;
83+
}
84+
2585
/**
2686
* Generates a static HTML page with a meta refresh tag to redirect the user to a specified URL.
2787
*
2888
* This function creates a simple HTML page that performs a redirect using a meta tag.
2989
* It includes a fallback link in case the meta-refresh doesn't work.
3090
*
91+
* The provided URL is HTML-escaped before being interpolated.
92+
*
3193
* @param url - The URL to which the page should redirect.
3294
* @returns The HTML content of the static redirect page.
3395
*/
3496
export function generateRedirectStaticPage(url: string): string {
97+
const escapedUrl = escapeHtml(url);
98+
3599
return `
36100
<!DOCTYPE html>
37101
<html>
38102
<head>
39103
<meta charset="utf-8">
40104
<title>Redirecting</title>
41-
<meta http-equiv="refresh" content="0; url=${url}">
105+
<meta http-equiv="refresh" content="0; url=${escapedUrl}">
42106
</head>
43107
<body>
44-
<pre>Redirecting to <a href="${url}">${url}</a></pre>
108+
<pre>Redirecting to <a href="${escapedUrl}">${escapedUrl}</a></pre>
45109
</body>
46110
</html>
47111
`.trim();

packages/angular/ssr/src/routes/ng-routes.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ import { Console } from '../console';
3030
import { AngularAppManifest, getAngularAppManifest } from '../manifest';
3131
import { AngularBootstrap, isNgModule } from '../utils/ng';
3232
import { promiseWithAbort } from '../utils/promise';
33-
import { VALID_REDIRECT_RESPONSE_CODES, isValidRedirectResponseCode } from '../utils/redirect';
33+
import {
34+
VALID_REDIRECT_RESPONSE_CODES,
35+
isValidRedirectResponseCode,
36+
validateUrlForStaticEmission,
37+
} from '../utils/redirect';
3438
import { addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
3539
import {
3640
PrerenderFallback,
@@ -525,6 +529,16 @@ function handlePrerenderParamsReplacement(
525529
);
526530
}
527531

532+
const invalidValueReason = validateUrlForStaticEmission(value);
533+
if (invalidValueReason) {
534+
throw new Error(
535+
`The 'getPrerenderParams' function defined for the '${stripLeadingSlash(currentRoutePath)}' route ` +
536+
`returned an unsafe value for parameter '${parameterName}': ${invalidValueReason} ` +
537+
`Such values would be embedded verbatim in generated URLs and static HTML, ` +
538+
`which can lead to HTML injection. Percent-encode the value or sanitize the source.`,
539+
);
540+
}
541+
528542
return parameterName === '**' ? `/${value}` : value;
529543
};
530544
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,47 @@ export function isValidRedirectResponseCode(code: number): boolean {
2121
return VALID_REDIRECT_RESPONSE_CODES.has(code);
2222
}
2323

24+
/**
25+
* Characters that must never appear in a statically-emitted redirect target or
26+
* in a value substituted into a prerendered URL. They are rejected during route
27+
* extraction so they cannot break out of generated HTML contexts or smuggle
28+
* markup through the route path.
29+
*
30+
* - C0 controls and DEL (`\u0000`-`\u001F`, `\u007F`) and whitespace
31+
* - Backslash (`\`) - not a valid URL path separator
32+
* - HTML-significant characters: `<`, `>`, `"`, `'`, `` ` ``
33+
*/
34+
const UNSAFE_URL_CHARACTERS_REGEXP = /[\\<>"'`]/;
35+
36+
function hasUnsafeUrlCharacters(value: string): boolean {
37+
for (let index = 0; index < value.length; index++) {
38+
const characterCode = value.charCodeAt(index);
39+
if (characterCode <= 0x20 || characterCode === 0x7f) {
40+
return true;
41+
}
42+
}
43+
44+
return UNSAFE_URL_CHARACTERS_REGEXP.test(value);
45+
}
46+
47+
/**
48+
* Validates that the given value is safe to embed in a generated redirect page
49+
* or to use as a prerendered URL path segment.
50+
*
51+
* Returns `undefined` when the value is safe, otherwise returns a human-readable
52+
* error message describing why it was rejected.
53+
*/
54+
export function validateUrlForStaticEmission(value: string): string | undefined {
55+
if (hasUnsafeUrlCharacters(value)) {
56+
return (
57+
`the value '${value}' contains characters that are not allowed in a statically ` +
58+
`emitted URL (control characters, whitespace, backslash, or HTML-significant characters).`
59+
);
60+
}
61+
62+
return undefined;
63+
}
64+
2465
/**
2566
* Creates an HTTP redirect response with a specified location and status code.
2667
*

packages/angular/ssr/test/routes/ng-routes_spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,62 @@ describe('extractRoutesAndCreateRouteTree', () => {
102102
);
103103
});
104104

105+
it("should error when 'getPrerenderParams' returns a value containing HTML-significant characters", async () => {
106+
setAngularAppTestingManifest(
107+
[{ path: 'store/:tenant/legacy/:slug', component: DummyComponent }],
108+
[
109+
{
110+
path: 'store/:tenant/legacy/:slug',
111+
renderMode: RenderMode.Prerender,
112+
fallback: PrerenderFallback.None,
113+
async getPrerenderParams() {
114+
return [
115+
{
116+
tenant: `x"><img src=x onerror=console.log("XSS_PARENT_IMG")>`,
117+
slug: 'old',
118+
},
119+
];
120+
},
121+
},
122+
],
123+
);
124+
125+
const { errors } = await extractRoutesAndCreateRouteTree({
126+
url,
127+
invokeGetPrerenderParams: true,
128+
});
129+
130+
expect(errors[0]).toContain(
131+
`the 'store/:tenant/legacy/:slug' route ` +
132+
`returned an unsafe value for parameter 'tenant'`,
133+
);
134+
});
135+
136+
it("should error when 'getPrerenderParams' returns a value containing control characters", async () => {
137+
setAngularAppTestingManifest(
138+
[{ path: 'docs/:id', component: DummyComponent }],
139+
[
140+
{
141+
path: 'docs/:id',
142+
renderMode: RenderMode.Prerender,
143+
fallback: PrerenderFallback.None,
144+
async getPrerenderParams() {
145+
return [{ id: 'a b' }];
146+
},
147+
},
148+
],
149+
);
150+
151+
const { errors } = await extractRoutesAndCreateRouteTree({
152+
url,
153+
invokeGetPrerenderParams: true,
154+
});
155+
156+
expect(errors[0]).toContain(
157+
`the 'docs/:id' route returned an unsafe value for parameter 'id'`,
158+
);
159+
});
160+
105161
it(`should not error when a catch-all route didn't match any Angular route`, async () => {
106162
setAngularAppTestingManifest(
107163
[{ path: 'home', component: DummyComponent }],

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { createRedirectResponse } from '../../src/utils/redirect';
9+
import { createRedirectResponse, validateUrlForStaticEmission } from '../../src/utils/redirect';
1010

1111
describe('Redirect Utils', () => {
1212
describe('createRedirectResponse', () => {
@@ -64,4 +64,16 @@ describe('Redirect Utils', () => {
6464
);
6565
});
6666
});
67+
68+
describe('validateUrlForStaticEmission', () => {
69+
it('should allow URL-safe values', () => {
70+
expect(validateUrlForStaticEmission('docs/page-1?from=ssg&next=/home')).toBeUndefined();
71+
});
72+
73+
it('should reject HTML-significant characters', () => {
74+
expect(validateUrlForStaticEmission('/docs"><script>alert(1)</script>')).toContain(
75+
'contains characters that are not allowed',
76+
);
77+
});
78+
});
6779
});

0 commit comments

Comments
 (0)