From ec61336338d5afc70ad87bf9b1a7dfd600496606 Mon Sep 17 00:00:00 2001 From: Jade Michael Thornton Date: Mon, 18 Oct 2021 09:46:21 -0500 Subject: [PATCH] dont crash on null byte in path --- lib/core/index.js | 206 +++++++++--------- ...ding.test.js => pathname-encoding.test.js} | 27 ++- 2 files changed, 135 insertions(+), 98 deletions(-) rename test/{showdir-pathname-encoding.test.js => pathname-encoding.test.js} (66%) diff --git a/lib/core/index.js b/lib/core/index.js index e72edbf9..7b2345a3 100644 --- a/lib/core/index.js +++ b/lib/core/index.js @@ -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; @@ -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) { @@ -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)) { @@ -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) { @@ -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); diff --git a/test/showdir-pathname-encoding.test.js b/test/pathname-encoding.test.js similarity index 66% rename from test/showdir-pathname-encoding.test.js rename to test/pathname-encoding.test.js index 89122f73..e87eb5d3 100644 --- a/test/showdir-pathname-encoding.test.js +++ b/test/pathname-encoding.test.js @@ -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; @@ -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({ @@ -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}/`); t.end();