plugins.serveStatic Fixes GH-1864/GH-1910

lib/plugins/static.js

@@ -4,16 +4,16 @@
 
 var fs = require('fs');
 var path = require('path');
-var escapeRE = require('escape-regexp-component');
 
 var assert = require('assert-plus');
 var mime = require('mime');
 var errors = require('restify-errors');
 
 ///--- Globals
 
+var BadRequestError = errors.BadRequestError;
+var ForbiddenError = errors.ForbiddenError;
 var MethodNotAllowedError = errors.MethodNotAllowedError;
-var NotAuthorizedError = errors.NotAuthorizedError;
 var ResourceNotFoundError = errors.ResourceNotFoundError;
 
 ///--- Functions
@@ -24,8 +24,9 @@ var ResourceNotFoundError = errors.ResourceNotFoundError;
  * @public
  * @function serveStatic
  * @param    {Object} options - an options object
- * @throws   {MethodNotAllowedError} |
- * @throws   {NotAuthorizedError}
+ * @throws   {BadRequestError}
+ * @throws   {ForbiddenError}
+ * @throws   {MethodNotAllowedError}
  * @throws   {ResourceNotFoundError}
  * @returns  {Function} Handler
  * @example
@@ -53,6 +54,9 @@ var ResourceNotFoundError = errors.ResourceNotFoundError;
  * directory that lacks a direct file match.
  * You can specify additional restrictions by passing in a `match` parameter,
  * which is just a `RegExp` to check against the requested file name.
+ * It is not matched against the request path.
+ * It is matched against the normalized unix file path including the
+ * `directory` option and depending on the other options.
  * Additionally, you may set the `charSet` parameter, which will append a
  * character set to the content-type detected by the plugin.
  * For example, `charSet: 'utf-8'` will result in HTML being served with a
@@ -89,8 +93,10 @@ function serveStatic(options) {
     assert.optionalString(opts.file, 'options.file');
     assert.bool(opts.appendRequestPath, 'options.appendRequestPath');
 
-    var p = path.normalize(opts.directory).replace(/\\/g, '/');
-    var re = new RegExp('^' + escapeRE(p) + '/?.*');
+    var docRoot = path.normalize(opts.directory).replaceAll('\\', '/');
+    if (!docRoot.endsWith('/')) {
+        docRoot += '/';
+    }
 
     function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
         if (typeof req.closed === 'function' && req.closed()) {
@@ -140,78 +146,91 @@ function serveStatic(options) {
     }
 
     function serveNormal(file, req, res, next) {
-        fs.stat(file, function fileStat(err, stats) {
-            if (!err && stats.isDirectory() && opts.default) {
-                // Serve an index.html page or similar
-                var filePath = path.join(file, opts.default);
-                fs.stat(filePath, function dirStat(dirErr, dirStats) {
-                    serveFileFromStats(
-                        filePath,
-                        dirErr,
-                        dirStats,
-                        false,
-                        req,
-                        res,
-                        next
-                    );
-                });
-            } else {
-                serveFileFromStats(file, err, stats, false, req, res, next);
-            }
-        });
+        try {
+            fs.stat(file, function fileStat(err, stats) {
+                if (!err && stats.isDirectory() && opts.default) {
+                    // Serve an index.html page or similar
+                    var filePath = path.join(file, opts.default);
+                    fs.stat(filePath, function dirStat(dirErr, dirStats) {
+                        serveFileFromStats(
+                            filePath,
+                            dirErr,
+                            dirStats,
+                            false,
+                            req,
+                            res,
+                            next
+                        );
+                    });
+                } else {
+                    serveFileFromStats(file, err, stats, false, req, res, next);
+                }
+            });
+        } catch (err) {
+            next(new BadRequestError(err, '%s', req.path()));
+            return;
+        }
+    }
+
+    function serveGzip(file, req, res, next) {
+        try {
+            fs.stat(file + '.gz', function stat(err, stats) {
+                if (!err) {
+                    res.setHeader('Content-Encoding', 'gzip');
+                    serveFileFromStats(file, err, stats, true, req, res, next);
+                } else {
+                    serveNormal(file, req, res, next);
+                }
+            });
+        } catch (err) {
+            next(new BadRequestError(err, '%s', req.path()));
+            return;
+        }
     }
 
     function serve(req, res, next) {
         var file;
 
+        if (req.method !== 'GET' && req.method !== 'HEAD') {
+            next(new MethodNotAllowedError('%s', req.method));
+            return;
+        }
+
         if (opts.file) {
-            //serves a direct file
-            file = path.join(opts.directory, decodeURIComponent(opts.file));
+            //serves a direct unchecked file
+            file = path.join(docRoot, opts.file);
         } else if (opts.appendRequestPath) {
-            file = path.join(opts.directory, decodeURIComponent(req.path()));
+            file = path.join(docRoot, decodeURIComponent(req.path()));
         } else {
-            var dirBasename = path.basename(opts.directory);
-            var reqpathBasename = path.basename(req.path());
+            var dirBasename = path.basename(docRoot);
+            var reqpathBasename = decodeURIComponent(path.basename(req.path()));
 
             if (
-                path.extname(req.path()) === '' &&
+                path.extname(reqpathBasename) === '' &&
                 dirBasename === reqpathBasename
             ) {
-                file = opts.directory;
+                file = docRoot;
             } else {
-                file = path.join(
-                    opts.directory,
-                    decodeURIComponent(path.basename(req.path()))
-                );
+                file = path.join(docRoot, reqpathBasename);
             }
         }
 
-        if (req.method !== 'GET' && req.method !== 'HEAD') {
-            next(new MethodNotAllowedError('%s', req.method));
+        // SAFETY CHECKS
+        var normalizedFile = path.normalize(file).replaceAll('\\', '/');
+        if (!normalizedFile.startsWith(docRoot)) {
+            next(new ForbiddenError('%s', req.path()));
             return;
         }
 
-        if (!re.test(file.replace(/\\/g, '/'))) {
-            next(new NotAuthorizedError('%s', req.path()));
-            return;
-        }
-
-        if (opts.match && !opts.match.test(file)) {
-            next(new NotAuthorizedError('%s', req.path()));
+        if (opts.match && !opts.match.test(normalizedFile)) {
+            next(new ForbiddenError('%s', req.path()));
             return;
         }
 
         if (opts.gzip && req.acceptsEncoding('gzip')) {
-            fs.stat(file + '.gz', function stat(err, stats) {
-                if (!err) {
-                    res.setHeader('Content-Encoding', 'gzip');
-                    serveFileFromStats(file, err, stats, true, req, res, next);
-                } else {
-                    serveNormal(file, req, res, next);
-                }
-            });
+            serveGzip(normalizedFile, req, res, next);
         } else {
-            serveNormal(file, req, res, next);
+            serveNormal(normalizedFile, req, res, next);
         }
     }