Skip to content

Commit

Permalink
Merge pull request #753 from http-party/null-byte-crash
Browse files Browse the repository at this point in the history 
Don't crash when file path errors
  • Loading branch information
@thornjad
thornjad committed Oct 18, 2021
2 parents 2b99fa5 + ec61336 commit 1e89681
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 98 deletions.
206 changes: 109 additions & 97 deletions lib/core/index.js
View file
Expand Up @@ -28,7 +28,7 @@ function decodePathname(pathname) {
return piece;
}).join('/'));
return process.platform === 'win32'
? normalized.replace(/\\/g, '/') : normalized;
? normalized.replace(/\\/g, '/') : normalized;
}

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

return headers && headers['accept-encoding'] &&
headers['accept-encoding']
.split(',')
.some(el => ['*', 'compress', 'gzip', 'deflate'].indexOf(el.trim()) !== -1)
;
.split(',')
.some(el => ['*', 'compress', 'gzip', 'deflate'].indexOf(el.trim()) !== -1)
;
}

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


function statFile() {
fs.stat(file, (err, stat) => {
if (err && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) {
if (req.statusCode === 404) {
// This means we're already trying ./404.html and can not find it.
// So send plain text response with 404 status code
status[404](res, next);
} else if (!path.extname(parsed.pathname).length && defaultExt) {
// If there is no file extension in the path and we have a default
// extension try filename and default extension combination before rendering 404.html.
middleware({
url: `${parsed.pathname}.${defaultExt}${(parsed.search) ? parsed.search : ''}`,
headers: req.headers,
}, res, next);
} else {
// Try to serve default ./404.html
const rawUrl = (handleError ? `/${path.join(baseDir, `404.${defaultExt}`)}` : req.url);
const encodedUrl = ensureUriEncoded(rawUrl);
middleware({
url: encodedUrl,
headers: req.headers,
statusCode: 404,
}, res, next);
}
} else if (err) {
status[500](res, next, { error: err });
} else if (stat.isDirectory()) {
if (!autoIndex && !opts.showDir) {
status[404](res, next);
return;
}

try {
fs.stat(file, (err, stat) => {
if (err && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) {
if (req.statusCode === 404) {
// This means we're already trying ./404.html and can not find it.
// So send plain text response with 404 status code
status[404](res, next);
} else if (!path.extname(parsed.pathname).length && defaultExt) {
// If there is no file extension in the path and we have a default
// extension try filename and default extension combination before rendering 404.html.
middleware({
url: `${parsed.pathname}.${defaultExt}${(parsed.search) ? parsed.search : ''}`,
headers: req.headers,
}, res, next);
} else {
// Try to serve default ./404.html
const rawUrl = (handleError ? `/${path.join(baseDir, `404.${defaultExt}`)}` : req.url);
const encodedUrl = ensureUriEncoded(rawUrl);
middleware({
url: encodedUrl,
headers: req.headers,
statusCode: 404,
}, res, next);
}
} else if (err) {
status[500](res, next, { error: err });
} else if (stat.isDirectory()) {
if (!autoIndex && !opts.showDir) {
status[404](res, next);
return;
}

// 302 to / if necessary
if (!pathname.match(/\/$/)) {
res.statusCode = 302;
const q = parsed.query ? `?${parsed.query}` : '';
res.setHeader(
'location',
ensureUriEncoded(`${parsed.pathname}/${q}`)
);
res.end();
return;
}

if (autoIndex) {
middleware({
url: urlJoin(
encodeURIComponent(pathname),
`/index.${defaultExt}`
),
headers: req.headers,
}, res, (autoIndexError) => {
if (autoIndexError) {
status[500](res, next, { error: autoIndexError });
return;
}
if (opts.showDir) {
showDir(opts, stat)(req, res);
return;
}
// 302 to / if necessary
if (!pathname.match(/\/$/)) {
res.statusCode = 302;
const q = parsed.query ? `?${parsed.query}` : '';
res.setHeader(
'location',
ensureUriEncoded(`${parsed.pathname}/${q}`)
);
res.end();
return;
}

status[403](res, next);
});
return;
}
if (autoIndex) {
middleware({
url: urlJoin(
encodeURIComponent(pathname),
`/index.${defaultExt}`
),
headers: req.headers,
}, res, (autoIndexError) => {
if (autoIndexError) {
status[500](res, next, { error: autoIndexError });
return;
}
if (opts.showDir) {
showDir(opts, stat)(req, res);
return;
}

status[403](res, next);
});
return;
}

if (opts.showDir) {
showDir(opts, stat)(req, res);
if (opts.showDir) {
showDir(opts, stat)(req, res);
}
} else {
serve(stat);
}
} else {
serve(stat);
}
});
});
} catch (err) {
status[500](res, next, { error: err.message });
}
}

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

// serve gzip file if exists and is valid
function tryServeWithGzip() {
fs.stat(gzippedFile, (err, stat) => {
if (!err && stat.isFile()) {
hasGzipId12(gzippedFile, (gzipErr, isGzip) => {
if (!gzipErr && isGzip) {
file = gzippedFile;
serve(stat);
} else {
statFile();
}
});
} else {
statFile();
}
});
try {
fs.stat(gzippedFile, (err, stat) => {
if (!err && stat.isFile()) {
hasGzipId12(gzippedFile, (gzipErr, isGzip) => {
if (!gzipErr && isGzip) {
file = gzippedFile;
serve(stat);
} else {
statFile();
}
});
} else {
statFile();
}
});
} catch (err) {
status[500](res, next, { error: err.message });
}
}

// serve brotli file if exists, otherwise try gzip
function tryServeWithBrotli(shouldTryGzip) {
fs.stat(brotliFile, (err, stat) => {
if (!err && stat.isFile()) {
file = brotliFile;
serve(stat);
} else if (shouldTryGzip) {
tryServeWithGzip();
} else {
statFile();
}
});
try {
fs.stat(brotliFile, (err, stat) => {
if (!err && stat.isFile()) {
file = brotliFile;
serve(stat);
} else if (shouldTryGzip) {
tryServeWithGzip();
} else {
statFile();
}
});
} catch (err) {
status[500](res, next, { error: err.message });
}
}

const shouldTryBrotli = opts.brotli && shouldCompressBrotli(req);
Expand Down
27 changes: 26 additions & 1 deletion test/showdir-pathname-encoding.test.js → test/pathname-encoding.test.js
View file
Expand Up @@ -5,6 +5,7 @@ const ecstatic = require('../lib/core');
const http = require('http');
const request = require('request');
const path = require('path');
const portfinder = require('portfinder');

const test = tap.test;

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

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

test('NULL byte in request path does not crash server', (t) => {
portfinder.getPort((err, port) => {
const uri = `http://localhost:${port}${path.join('/', baseDir, '/%00')}`;
const server = http.createServer(
ecstatic({
root,
baseDir,
})
);

try {
server.listen(port, () => {
request.get({uri}, (err, res, body) => {
t.pass('server did not crash')
server.close();
t.end();
});
});
} catch (err) {
t.fail(err.toString());
}
});
});

test('remove test directory', (t) => {
fs.rmdirSync(`${root}/<dir>`);
t.end();
Expand Down

0 comments on commit 1e89681

Please sign in to comment.