Skip to content

Commit 7f9f044

Browse files
committed
refactor(@angular/ssr): guard against potential path traversals
This change updates to code to guard against a potential path traversal. More context about the reasoning behind this change can be found in https://buganizer.corp.google.com/issues/299878755#comment26 (cherry picked from commit 0f5fb09)
1 parent b5edaa0 commit 7f9f044

File tree

1 file changed

+27
-34
lines changed

1 file changed

+27
-34
lines changed

packages/angular/ssr/src/common-engine.ts

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { ApplicationRef, StaticProvider, Type } from '@angular/core';
1010
import { renderApplication, renderModule, ɵSERVER_CONTEXT } from '@angular/platform-server';
1111
import * as fs from 'node:fs';
12-
import { dirname, resolve } from 'node:path';
12+
import { dirname, join, normalize, resolve } from 'node:path';
1313
import { URL } from 'node:url';
1414
import { InlineCriticalCssProcessor, InlineCriticalCssResult } from './inline-css-processor';
1515
import {
@@ -117,32 +117,34 @@ export class CommonEngine {
117117
return undefined;
118118
}
119119

120-
const pathname = canParseUrl(url) ? new URL(url).pathname : url;
121-
// Remove leading forward slash.
122-
const pagePath = resolve(publicPath, pathname.substring(1), 'index.html');
123-
124-
if (pagePath !== resolve(documentFilePath)) {
125-
// View path doesn't match with prerender path.
126-
const pageIsSSG = this.pageIsSSG.get(pagePath);
127-
if (pageIsSSG === undefined) {
128-
if (await exists(pagePath)) {
129-
const content = await fs.promises.readFile(pagePath, 'utf-8');
130-
const isSSG = SSG_MARKER_REGEXP.test(content);
131-
this.pageIsSSG.set(pagePath, isSSG);
132-
133-
if (isSSG) {
134-
return content;
135-
}
136-
} else {
137-
this.pageIsSSG.set(pagePath, false);
138-
}
139-
} else if (pageIsSSG) {
140-
// Serve pre-rendered page.
141-
return fs.promises.readFile(pagePath, 'utf-8');
142-
}
120+
const { pathname } = new URL(url, 'resolve://');
121+
// Do not use `resolve` here as otherwise it can lead to path traversal vulnerability.
122+
// See: https://portswigger.net/web-security/file-path-traversal
123+
const pagePath = join(publicPath, pathname, 'index.html');
124+
125+
if (this.pageIsSSG.get(pagePath)) {
126+
// Serve pre-rendered page.
127+
return fs.promises.readFile(pagePath, 'utf-8');
128+
}
129+
130+
if (!pagePath.startsWith(normalize(publicPath))) {
131+
// Potential path traversal detected.
132+
return undefined;
133+
}
134+
135+
if (pagePath === resolve(documentFilePath) || !(await exists(pagePath))) {
136+
// View matches with prerender path or file does not exist.
137+
this.pageIsSSG.set(pagePath, false);
138+
139+
return undefined;
143140
}
144141

145-
return undefined;
142+
// Static file exists.
143+
const content = await fs.promises.readFile(pagePath, 'utf-8');
144+
const isSSG = SSG_MARKER_REGEXP.test(content);
145+
this.pageIsSSG.set(pagePath, isSSG);
146+
147+
return isSSG ? content : undefined;
146148
}
147149

148150
private async renderApplication(opts: CommonEngineRenderOptions): Promise<string> {
@@ -202,12 +204,3 @@ function isBootstrapFn(value: unknown): value is () => Promise<ApplicationRef> {
202204
// We can differentiate between a module and a bootstrap function by reading compiler-generated `ɵmod` static property:
203205
return typeof value === 'function' && !('ɵmod' in value);
204206
}
205-
206-
// The below can be removed in favor of URL.canParse() when Node.js 18 is dropped
207-
function canParseUrl(url: string): boolean {
208-
try {
209-
return !!new URL(url);
210-
} catch {
211-
return false;
212-
}
213-
}

0 commit comments

Comments
 (0)