Skip to content

Commit 189c4ac

Browse files
alexander-akaitalexander-akait
authored
fix(security): do not allow to read files above (#1779)
1 parent f3c62b8 commit 189c4ac

File tree

5 files changed

+225
-26
lines changed

5 files changed

+225
-26
lines changed

‎src/middleware.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
setHeaderForResponse,
1212
setStatusCode,
1313
send,
14+
sendError,
1415
} = require("./utils/compatibleAPI");
1516
const ready = require("./utils/ready");
1617

@@ -95,9 +96,12 @@ function wrapper(context) {
9596
}
9697

9798
async function processRequest() {
99+
/** @type {import("./utils/getFilenameFromUrl").Extra} */
100+
const extra = {};
98101
const filename = getFilenameFromUrl(
99102
context,
100-
/** @type {string} */ (req.url)
103+
/** @type {string} */ (req.url),
104+
extra
101105
);
102106

103107
if (!filename) {
@@ -106,6 +110,16 @@ function wrapper(context) {
106110
return;
107111
}
108112

113+
if (extra.errorCode) {
114+
if (extra.errorCode === 403) {
115+
context.logger.error(`Malicious path "${filename}".`);
116+
}
117+
118+
sendError(req, res, extra.errorCode);
119+
120+
return;
121+
}
122+
109123
let { headers } = context.options;
110124

111125
if (typeof headers === "function") {

‎src/utils/compatibleAPI.js

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,127 @@ function send(req, res, bufferOtStream, byteLength) {
155155
}
156156
}
157157

158+
/**
159+
* @template {ServerResponse} Response
160+
* @param {Response} res
161+
*/
162+
function clearHeadersForResponse(res) {
163+
const headers = getHeaderNames(res);
164+
165+
for (let i = 0; i < headers.length; i++) {
166+
res.removeHeader(headers[i]);
167+
}
168+
}
169+
170+
const matchHtmlRegExp = /["'&<>]/;
171+
172+
/**
173+
* @param {string} string raw HTML
174+
* @returns {string} escaped HTML
175+
*/
176+
function escapeHtml(string) {
177+
const str = `${string}`;
178+
const match = matchHtmlRegExp.exec(str);
179+
180+
if (!match) {
181+
return str;
182+
}
183+
184+
let escape;
185+
let html = "";
186+
let index = 0;
187+
let lastIndex = 0;
188+
189+
for ({ index } = match; index < str.length; index++) {
190+
switch (str.charCodeAt(index)) {
191+
// "
192+
case 34:
193+
escape = "&quot;";
194+
break;
195+
// &
196+
case 38:
197+
escape = "&amp;";
198+
break;
199+
// '
200+
case 39:
201+
escape = "&#39;";
202+
break;
203+
// <
204+
case 60:
205+
escape = "&lt;";
206+
break;
207+
// >
208+
case 62:
209+
escape = "&gt;";
210+
break;
211+
default:
212+
// eslint-disable-next-line no-continue
213+
continue;
214+
}
215+
216+
if (lastIndex !== index) {
217+
html += str.substring(lastIndex, index);
218+
}
219+
220+
lastIndex = index + 1;
221+
html += escape;
222+
}
223+
224+
return lastIndex !== index ? html + str.substring(lastIndex, index) : html;
225+
}
226+
227+
/** @type {Record<number, string>} */
228+
const statuses = {
229+
400: "Bad Request",
230+
403: "Forbidden",
231+
404: "Not Found",
232+
416: "Range Not Satisfiable",
233+
500: "Internal Server Error",
234+
};
235+
236+
/**
237+
* @template {IncomingMessage} Request
238+
* @template {ServerResponse} Response
239+
* @param {Request} req response
240+
* @param {Response} res response
241+
* @param {number} status status
242+
* @returns {void}
243+
*/
244+
function sendError(req, res, status) {
245+
const content = statuses[status] || String(status);
246+
const document = `<!DOCTYPE html>
247+
<html lang="en">
248+
<head>
249+
<meta charset="utf-8">
250+
<title>Error</title>
251+
</head>
252+
<body>
253+
<pre>${escapeHtml(content)}</pre>
254+
</body>
255+
</html>`;
256+
257+
// Clear existing headers
258+
clearHeadersForResponse(res);
259+
260+
// Send basic response
261+
setStatusCode(res, status);
262+
setHeaderForResponse(res, "Content-Type", "text/html; charset=utf-8");
263+
setHeaderForResponse(res, "Content-Security-Policy", "default-src 'none'");
264+
setHeaderForResponse(res, "X-Content-Type-Options", "nosniff");
265+
266+
const byteLength = Buffer.byteLength(document);
267+
268+
setHeaderForResponse(res, "Content-Length", byteLength);
269+
270+
res.end(document);
271+
}
272+
158273
module.exports = {
159274
getHeaderNames,
160275
getHeaderFromRequest,
161276
getHeaderFromResponse,
162277
setHeaderForResponse,
163278
setStatusCode,
164279
send,
280+
sendError,
165281
};

‎src/utils/getFilenameFromUrl.js

Lines changed: 74 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@ const getPaths = require("./getPaths");
1010
const cacheStore = new WeakMap();
1111

1212
/**
13+
* @template T
1314
* @param {Function} fn
14-
* @param {{ cache?: Map<any, any> }} [cache]
15+
* @param {{ cache?: Map<string, { data: T }> } | undefined} cache
16+
* @param {(value: T) => T} callback
1517
* @returns {any}
1618
*/
17-
const mem = (fn, { cache = new Map() } = {}) => {
19+
// @ts-ignore
20+
const mem = (fn, { cache = new Map() } = {}, callback) => {
1821
/**
1922
* @param {any} arguments_
2023
* @return {any}
@@ -27,7 +30,8 @@ const mem = (fn, { cache = new Map() } = {}) => {
2730
return cacheItem.data;
2831
}
2932

30-
const result = fn.apply(this, arguments_);
33+
let result = fn.apply(this, arguments_);
34+
result = callback(result);
3135

3236
cache.set(key, {
3337
data: result,
@@ -40,20 +44,52 @@ const mem = (fn, { cache = new Map() } = {}) => {
4044

4145
return memoized;
4246
};
43-
const memoizedParse = mem(parse);
47+
// eslint-disable-next-line no-undefined
48+
const memoizedParse = mem(parse, undefined, (value) => {
49+
if (value.pathname) {
50+
// eslint-disable-next-line no-param-reassign
51+
value.pathname = decode(value.pathname);
52+
}
53+
54+
return value;
55+
});
56+
57+
const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/;
58+
59+
/**
60+
* @typedef {Object} Extra
61+
* @property {import("fs").Stats=} stats
62+
* @property {number=} errorCode
63+
*/
64+
65+
/**
66+
* decodeURIComponent.
67+
*
68+
* Allows V8 to only deoptimize this fn instead of all of send().
69+
*
70+
* @param {string} input
71+
* @returns {string}
72+
*/
73+
74+
function decode(input) {
75+
return querystring.unescape(input);
76+
}
4477

4578
/**
4679
* @template {IncomingMessage} Request
4780
* @template {ServerResponse} Response
4881
* @param {import("../index.js").Context<Request, Response>} context
4982
* @param {string} url
83+
* @param {Extra=} extra
5084
* @returns {string | undefined}
5185
*/
52-
function getFilenameFromUrl(context, url) {
86+
function getFilenameFromUrl(context, url, extra = {}) {
5387
const { options } = context;
5488
const paths = getPaths(context);
5589

90+
/** @type {string | undefined} */
5691
let foundFilename;
92+
/** @type {URL} */
5793
let urlObject;
5894

5995
try {
@@ -64,7 +100,9 @@ function getFilenameFromUrl(context, url) {
64100
}
65101

66102
for (const { publicPath, outputPath } of paths) {
103+
/** @type {string | undefined} */
67104
let filename;
105+
/** @type {URL} */
68106
let publicPathObject;
69107

70108
try {
@@ -78,39 +116,51 @@ function getFilenameFromUrl(context, url) {
78116
continue;
79117
}
80118

81-
if (
82-
urlObject.pathname &&
83-
urlObject.pathname.startsWith(publicPathObject.pathname)
84-
) {
85-
filename = outputPath;
119+
const { pathname } = urlObject;
120+
const { pathname: publicPathPathname } = publicPathObject;
86121

87-
// Strip the `pathname` property from the `publicPath` option from the start of requested url
88-
// `/complex/foo.js` => `foo.js`
89-
const pathname = urlObject.pathname.slice(
90-
publicPathObject.pathname.length
91-
);
122+
if (pathname && pathname.startsWith(publicPathPathname)) {
123+
// Null byte(s)
124+
if (pathname.includes("\0")) {
125+
// eslint-disable-next-line no-param-reassign
126+
extra.errorCode = 400;
127+
128+
return;
129+
}
130+
131+
// ".." is malicious
132+
if (UP_PATH_REGEXP.test(path.normalize(`./${pathname}`))) {
133+
// eslint-disable-next-line no-param-reassign
134+
extra.errorCode = 403;
92135

93-
if (pathname) {
94-
filename = path.join(outputPath, querystring.unescape(pathname));
136+
return;
95137
}
96138

97-
let fsStats;
139+
// Strip the `pathname` property from the `publicPath` option from the start of requested url
140+
// `/complex/foo.js` => `foo.js`
141+
// and add outputPath
142+
// `foo.js` => `/home/user/my-project/dist/foo.js`
143+
filename = path.join(
144+
outputPath,
145+
pathname.slice(publicPathPathname.length)
146+
);
98147

99148
try {
100-
fsStats =
149+
// eslint-disable-next-line no-param-reassign
150+
extra.stats =
101151
/** @type {import("fs").statSync} */
102152
(context.outputFileSystem.statSync)(filename);
103153
} catch (_ignoreError) {
104154
// eslint-disable-next-line no-continue
105155
continue;
106156
}
107157

108-
if (fsStats.isFile()) {
158+
if (extra.stats.isFile()) {
109159
foundFilename = filename;
110160

111161
break;
112162
} else if (
113-
fsStats.isDirectory() &&
163+
extra.stats.isDirectory() &&
114164
(typeof options.index === "undefined" || options.index)
115165
) {
116166
const indexValue =
@@ -122,15 +172,16 @@ function getFilenameFromUrl(context, url) {
122172
filename = path.join(filename, indexValue);
123173

124174
try {
125-
fsStats =
175+
// eslint-disable-next-line no-param-reassign
176+
extra.stats =
126177
/** @type {import("fs").statSync} */
127178
(context.outputFileSystem.statSync)(filename);
128179
} catch (__ignoreError) {
129180
// eslint-disable-next-line no-continue
130181
continue;
131182
}
132183

133-
if (fsStats.isFile()) {
184+
if (extra.stats.isFile()) {
134185
foundFilename = filename;
135186

136187
break;

‎types/utils/compatibleAPI.d.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,15 @@ export function send<
8484
bufferOtStream: string | Buffer | import("fs").ReadStream,
8585
byteLength: number
8686
): void;
87+
/**
88+
* @template {IncomingMessage} Request
89+
* @template {ServerResponse} Response
90+
* @param {Request} req response
91+
* @param {Response} res response
92+
* @param {number} status status
93+
* @returns {void}
94+
*/
95+
export function sendError<
96+
Request_1 extends import("http").IncomingMessage,
97+
Response_1 extends import("../index.js").ServerResponse
98+
>(req: Request_1, res: Response_1, status: number): void;

‎types/utils/getFilenameFromUrl.d.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,23 @@ export = getFilenameFromUrl;
55
* @template {ServerResponse} Response
66
* @param {import("../index.js").Context<Request, Response>} context
77
* @param {string} url
8+
* @param {Extra=} extra
89
* @returns {string | undefined}
910
*/
1011
declare function getFilenameFromUrl<
1112
Request_1 extends import("http").IncomingMessage,
1213
Response_1 extends import("../index.js").ServerResponse
1314
>(
1415
context: import("../index.js").Context<Request_1, Response_1>,
15-
url: string
16+
url: string,
17+
extra?: Extra | undefined
1618
): string | undefined;
1719
declare namespace getFilenameFromUrl {
18-
export { IncomingMessage, ServerResponse };
20+
export { Extra, IncomingMessage, ServerResponse };
1921
}
22+
type Extra = {
23+
stats?: import("fs").Stats | undefined;
24+
errorCode?: number | undefined;
25+
};
2026
type IncomingMessage = import("../index.js").IncomingMessage;
2127
type ServerResponse = import("../index.js").ServerResponse;

0 commit comments

Comments
 (0)