feat: fix path traversal

lib/plugins/static.js

@@ -4,14 +4,15 @@
 
 var fs = require('fs');
 var path = require('path');
-var escapeRE = require('escape-regexp-component');
 
 var assert = require('assert-plus');
+var escapeRE = require('escape-regexp-component');
 var mime = require('mime');
 var errors = require('restify-errors');
 
 ///--- Globals
 
+var BadRequestError = errors.BadRequestError;
 var MethodNotAllowedError = errors.MethodNotAllowedError;
 var NotAuthorizedError = errors.NotAuthorizedError;
 var ResourceNotFoundError = errors.ResourceNotFoundError;
@@ -24,6 +25,7 @@ var ResourceNotFoundError = errors.ResourceNotFoundError;
  * @public
  * @function serveStatic
  * @param    {Object} options - an options object
+ * @throws   {BadRequestError} |
  * @throws   {MethodNotAllowedError} |
  * @throws   {NotAuthorizedError}
  * @throws   {ResourceNotFoundError}
@@ -89,8 +91,20 @@ 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 resolvedDirectory = path.resolve(opts.directory);
+    // `$` can match before a final newline, so use a true end-of-string check.
+    var directoryRE = new RegExp(
+        '^' +
+            escapeRE(resolvedDirectory) +
+            (resolvedDirectory.endsWith(path.sep)
+                ? ''
+                : '(?:' + escapeRE(path.sep) + '|(?![\\s\\S]))'),
+        process.platform === 'win32' ? 'i' : ''
+    );
+
+    function isPathInDirectory(file) {
+        return directoryRE.test(path.resolve(file));
+    }
 
     function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
         if (
@@ -194,7 +208,12 @@ function serveStatic(options) {
             return;
         }
 
-        if (!re.test(file.replace(/\\/g, '/'))) {
+        if (file.indexOf('\0') !== -1) {
+            next(new BadRequestError('Invalid path'));
+            return;
+        }
+
+        if (!isPathInDirectory(file)) {
             next(new NotAuthorizedError('%s', req.path()));
             return;
         }

test/plugins/static.test.js

@@ -244,6 +244,64 @@ describe('static resource plugin', function() {
         });
     });
 
+    it('static rejects traversal into a sibling directory', function(done) {
+        var tmpPath = path.join(process.cwd(), '.tmp', 'public');
+        var siblingPath = tmpPath + '-secret';
+        var secretFile = path.join(siblingPath, 'secret.json');
+
+        mkdirp.sync(tmpPath);
+        mkdirp.sync(siblingPath);
+        fs.writeFileSync(secretFile, '{"secret": true}');
+        DIRS_TO_DELETE.push(path.dirname(tmpPath));
+
+        SERVER.get('/*', restify.plugins.serveStatic({ directory: tmpPath }));
+
+        CLIENT.get(
+            '/..%2f' + path.basename(siblingPath) + '/secret.json',
+            function(err) {
+                assert.ok(err);
+                assert.equal(err.statusCode, 403);
+                assert.equal(err.restCode, 'NotAuthorized');
+                done();
+            }
+        );
+    });
+
+    it('static rejects a sibling ending in a newline', function(done) {
+        var tmpPath = path.join(process.cwd(), '.tmp', 'public');
+
+        SERVER.get('/*', restify.plugins.serveStatic({ directory: tmpPath }));
+
+        CLIENT.get('/..%2fpublic%0a/secret.json', function(err) {
+            assert.ok(err);
+            assert.strictEqual(err.statusCode, 403);
+            assert.strictEqual(err.restCode, 'NotAuthorized');
+            done();
+        });
+    });
+
+    it('static rejects NUL bytes without crashing the server', function(done) {
+        var tmpPath = path.join(process.cwd(), '.tmp', 'nul-byte');
+
+        mkdirp.sync(tmpPath);
+        fs.writeFileSync(path.join(tmpPath, 'ok.json'), '{"ok": true}');
+        DIRS_TO_DELETE.push(tmpPath);
+
+        SERVER.get('/*', restify.plugins.serveStatic({ directory: tmpPath }));
+
+        CLIENT.get('/%00', function(err) {
+            assert.ok(err);
+            assert.strictEqual(err.statusCode, 400);
+            assert.strictEqual(err.restCode, 'BadRequest');
+
+            CLIENT.get('/ok.json', function(nextErr, req, res, obj) {
+                assert.ifError(nextErr);
+                assert.deepEqual(obj, { ok: true });
+                done();
+            });
+        });
+    });
+
     // eslint-disable-next-line
     it('GH-1382 static responds 404 for missing file with percent-codes', function(done) {
         var p = '/public/no-%22such-file.json';