Skip to content

Commit 1e89681

Browse files
thornjadthornjad
authored
Merge pull request #753 from http-party/null-byte-crash
Don't crash when file path errors
2 parents 2b99fa5 + ec61336 commit 1e89681

File tree

2 files changed

+135
-98
lines changed

2 files changed

+135
-98
lines changed

‎lib/core/index.js‎

Lines changed: 109 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function decodePathname(pathname) {
2828
return piece;
2929
}).join('/'));
3030
return process.platform === 'win32'
31-
? normalized.replace(/\\/g, '/') : normalized;
31+
? normalized.replace(/\\/g, '/') : normalized;
3232
}
3333

3434
const nonUrlSafeCharsRgx = /[\x00-\x1F\x20\x7F-\uFFFF]+/g;
@@ -43,9 +43,9 @@ function shouldCompressGzip(req) {
4343

4444
return headers && headers['accept-encoding'] &&
4545
headers['accept-encoding']
46-
.split(',')
47-
.some(el => ['*', 'compress', 'gzip', 'deflate'].indexOf(el.trim()) !== -1)
48-
;
46+
.split(',')
47+
.some(el => ['*', 'compress', 'gzip', 'deflate'].indexOf(el.trim()) !== -1)
48+
;
4949
}
5050

5151
function shouldCompressBrotli(req) {
@@ -164,7 +164,7 @@ module.exports = function createMiddleware(_dir, _options) {
164164
// Do a strong or weak etag comparison based on setting
165165
// https://www.ietf.org/rfc/rfc2616.txt Section 13.3.3
166166
if (opts.weakCompare && clientEtag !== serverEtag
167-
&& clientEtag !== `W/${serverEtag}` && `W/${clientEtag}` !== serverEtag) {
167+
&& clientEtag !== `W/${serverEtag}` && `W/${clientEtag}` !== serverEtag) {
168168
return false;
169169
}
170170
if (!opts.weakCompare && (clientEtag !== serverEtag || clientEtag.indexOf('W/') === 0)) {
@@ -330,79 +330,83 @@ module.exports = function createMiddleware(_dir, _options) {
330330

331331

332332
function statFile() {
333-
fs.stat(file, (err, stat) => {
334-
if (err && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) {
335-
if (req.statusCode === 404) {
336-
// This means we're already trying ./404.html and can not find it.
337-
// So send plain text response with 404 status code
338-
status[404](res, next);
339-
} else if (!path.extname(parsed.pathname).length && defaultExt) {
340-
// If there is no file extension in the path and we have a default
341-
// extension try filename and default extension combination before rendering 404.html.
342-
middleware({
343-
url: `${parsed.pathname}.${defaultExt}${(parsed.search) ? parsed.search : ''}`,
344-
headers: req.headers,
345-
}, res, next);
346-
} else {
347-
// Try to serve default ./404.html
348-
const rawUrl = (handleError ? `/${path.join(baseDir, `404.${defaultExt}`)}` : req.url);
349-
const encodedUrl = ensureUriEncoded(rawUrl);
350-
middleware({
351-
url: encodedUrl,
352-
headers: req.headers,
353-
statusCode: 404,
354-
}, res, next);
355-
}
356-
} else if (err) {
357-
status[500](res, next, { error: err });
358-
} else if (stat.isDirectory()) {
359-
if (!autoIndex && !opts.showDir) {
360-
status[404](res, next);
361-
return;
362-
}
363-
333+
try {
334+
fs.stat(file, (err, stat) => {
335+
if (err && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) {
336+
if (req.statusCode === 404) {
337+
// This means we're already trying ./404.html and can not find it.
338+
// So send plain text response with 404 status code
339+
status[404](res, next);
340+
} else if (!path.extname(parsed.pathname).length && defaultExt) {
341+
// If there is no file extension in the path and we have a default
342+
// extension try filename and default extension combination before rendering 404.html.
343+
middleware({
344+
url: `${parsed.pathname}.${defaultExt}${(parsed.search) ? parsed.search : ''}`,
345+
headers: req.headers,
346+
}, res, next);
347+
} else {
348+
// Try to serve default ./404.html
349+
const rawUrl = (handleError ? `/${path.join(baseDir, `404.${defaultExt}`)}` : req.url);
350+
const encodedUrl = ensureUriEncoded(rawUrl);
351+
middleware({
352+
url: encodedUrl,
353+
headers: req.headers,
354+
statusCode: 404,
355+
}, res, next);
356+
}
357+
} else if (err) {
358+
status[500](res, next, { error: err });
359+
} else if (stat.isDirectory()) {
360+
if (!autoIndex && !opts.showDir) {
361+
status[404](res, next);
362+
return;
363+
}
364364

365-
// 302 to / if necessary
366-
if (!pathname.match(/\/$/)) {
367-
res.statusCode = 302;
368-
const q = parsed.query ? `?${parsed.query}` : '';
369-
res.setHeader(
370-
'location',
371-
ensureUriEncoded(`${parsed.pathname}/${q}`)
372-
);
373-
res.end();
374-
return;
375-
}
376365

377-
if (autoIndex) {
378-
middleware({
379-
url: urlJoin(
380-
encodeURIComponent(pathname),
381-
`/index.${defaultExt}`
382-
),
383-
headers: req.headers,
384-
}, res, (autoIndexError) => {
385-
if (autoIndexError) {
386-
status[500](res, next, { error: autoIndexError });
387-
return;
388-
}
389-
if (opts.showDir) {
390-
showDir(opts, stat)(req, res);
391-
return;
392-
}
366+
// 302 to / if necessary
367+
if (!pathname.match(/\/$/)) {
368+
res.statusCode = 302;
369+
const q = parsed.query ? `?${parsed.query}` : '';
370+
res.setHeader(
371+
'location',
372+
ensureUriEncoded(`${parsed.pathname}/${q}`)
373+
);
374+
res.end();
375+
return;
376+
}
393377

394-
status[403](res, next);
395-
});
396-
return;
397-
}
378+
if (autoIndex) {
379+
middleware({
380+
url: urlJoin(
381+
encodeURIComponent(pathname),
382+
`/index.${defaultExt}`
383+
),
384+
headers: req.headers,
385+
}, res, (autoIndexError) => {
386+
if (autoIndexError) {
387+
status[500](res, next, { error: autoIndexError });
388+
return;
389+
}
390+
if (opts.showDir) {
391+
showDir(opts, stat)(req, res);
392+
return;
393+
}
394+
395+
status[403](res, next);
396+
});
397+
return;
398+
}
398399

399-
if (opts.showDir) {
400-
showDir(opts, stat)(req, res);
400+
if (opts.showDir) {
401+
showDir(opts, stat)(req, res);
402+
}
403+
} else {
404+
serve(stat);
401405
}
402-
} else {
403-
serve(stat);
404-
}
405-
});
406+
});
407+
} catch (err) {
408+
status[500](res, next, { error: err.message });
409+
}
406410
}
407411

408412
function isTextFile(mimeType) {
@@ -411,34 +415,42 @@ module.exports = function createMiddleware(_dir, _options) {
411415

412416
// serve gzip file if exists and is valid
413417
function tryServeWithGzip() {
414-
fs.stat(gzippedFile, (err, stat) => {
415-
if (!err && stat.isFile()) {
416-
hasGzipId12(gzippedFile, (gzipErr, isGzip) => {
417-
if (!gzipErr && isGzip) {
418-
file = gzippedFile;
419-
serve(stat);
420-
} else {
421-
statFile();
422-
}
423-
});
424-
} else {
425-
statFile();
426-
}
427-
});
418+
try {
419+
fs.stat(gzippedFile, (err, stat) => {
420+
if (!err && stat.isFile()) {
421+
hasGzipId12(gzippedFile, (gzipErr, isGzip) => {
422+
if (!gzipErr && isGzip) {
423+
file = gzippedFile;
424+
serve(stat);
425+
} else {
426+
statFile();
427+
}
428+
});
429+
} else {
430+
statFile();
431+
}
432+
});
433+
} catch (err) {
434+
status[500](res, next, { error: err.message });
435+
}
428436
}
429437

430438
// serve brotli file if exists, otherwise try gzip
431439
function tryServeWithBrotli(shouldTryGzip) {
432-
fs.stat(brotliFile, (err, stat) => {
433-
if (!err && stat.isFile()) {
434-
file = brotliFile;
435-
serve(stat);
436-
} else if (shouldTryGzip) {
437-
tryServeWithGzip();
438-
} else {
439-
statFile();
440-
}
441-
});
440+
try {
441+
fs.stat(brotliFile, (err, stat) => {
442+
if (!err && stat.isFile()) {
443+
file = brotliFile;
444+
serve(stat);
445+
} else if (shouldTryGzip) {
446+
tryServeWithGzip();
447+
} else {
448+
statFile();
449+
}
450+
});
451+
} catch (err) {
452+
status[500](res, next, { error: err.message });
453+
}
442454
}
443455

444456
const shouldTryBrotli = opts.brotli && shouldCompressBrotli(req);

‎test/showdir-pathname-encoding.test.js‎ renamed to ‎test/pathname-encoding.test.js‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const ecstatic = require('../lib/core');
55
const http = require('http');
66
const request = require('request');
77
const path = require('path');
8+
const portfinder = require('portfinder');
89

910
const test = tap.test;
1011

@@ -24,7 +25,7 @@ test('create test directory', (t) => {
2425
});
2526

2627
test('directory listing with pathname including HTML characters', (t) => {
27-
require('portfinder').getPort((err, port) => {
28+
portfinder.getPort((err, port) => {
2829
const uri = `http://localhost:${port}${path.join('/', baseDir, '/%3Cdir%3E')}`;
2930
const server = http.createServer(
3031
ecstatic({
@@ -48,6 +49,30 @@ test('directory listing with pathname including HTML characters', (t) => {
4849
});
4950
});
5051

52+
test('NULL byte in request path does not crash server', (t) => {
53+
portfinder.getPort((err, port) => {
54+
const uri = `http://localhost:${port}${path.join('/', baseDir, '/%00')}`;
55+
const server = http.createServer(
56+
ecstatic({
57+
root,
58+
baseDir,
59+
})
60+
);
61+
62+
try {
63+
server.listen(port, () => {
64+
request.get({uri}, (err, res, body) => {
65+
t.pass('server did not crash')
66+
server.close();
67+
t.end();
68+
});
69+
});
70+
} catch (err) {
71+
t.fail(err.toString());
72+
}
73+
});
74+
});
75+
5176
test('remove test directory', (t) => {
5277
fs.rmdirSync(`${root}/<dir>`);
5378
t.end();

0 commit comments

Comments
 (0)