CVE-2020-26299: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: ftp-srv is an open-source FTP server designed to be simple yet configurable. In ftp-srv before version 4.4.0 there is a path-traversal vulnerability. Clients of FTP servers utilizing ftp-srv hosted on Windows machines can escape the FTP user's defined root folder using the expected FTP commands, for example, CWD and UPDR. When windows separators exist within the path (`\`), `path.resolve` leaves the upper pointers intact and allows the user to move beyond the root folder defined for that user. We did not take that into account when creating the path resolve function. The issue is patched in version 4.4.0 (commit 457b859450a37cba10ff3c431eb4aa67771122e3).
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.6
Published: 2021-02-10T18:15Z
Repository: https://github.com/autovance/ftp-srv
Language: JavaScript
Files changed: src/fs.js, test/fs.spec.js, test/start.js
Commit date: 2020-12-16 10:19:28 -0700
Commit message: fix(fs): check resolved path against root (#224)

* fix(fs): check resolved path against root

This should prevent paths from being resolved above the root.

Should affect all commands that utilize the FS functions.

Fixes #167

* test: use __dirname for relative certs

* fix: improve path resolution

* chore: remove unused package

* fix: normalize resolve path if absolute

Otherwise join will normalize

Co-authored-by: Tyler Stewart <tyler@autovance.com>
Diff: diff --git a/src/fs.js b/src/fs.js
index 3226d2e..697cab4 100644
--- a/src/fs.js
+++ b/src/fs.js
@@ -4,35 +4,40 @@ const uuid = require('uuid');
 const Promise = require('bluebird');
 const {createReadStream, createWriteStream, constants} = require('fs');
 const fsAsync = require('./helpers/fs-async');
 const errors = require('./errors');
 
+const UNIX_SEP_REGEX = /\//g;
+const WIN_SEP_REGEX = /\\/g;
+
 class FileSystem {
   constructor(connection, {root, cwd} = {}) {
     this.connection = connection;
-    this.cwd = nodePath.normalize(cwd ? nodePath.join(nodePath.sep, cwd) : nodePath.sep);
+    this.cwd = nodePath.normalize((cwd || '/').replace(WIN_SEP_REGEX, '/'));
     this._root = nodePath.resolve(root || process.cwd());
   }
 
   get root() {
     return this._root;
   }
 
   _resolvePath(path = '.') {
-    const clientPath = (() => {
-      path = nodePath.normalize(path);
-      if (nodePath.isAbsolute(path)) {
-        return nodePath.join(path);
-      } else {
-        return nodePath.join(this.cwd, path);
-      }
-    })();
-
-    const fsPath = (() => {
-      const resolvedPath = nodePath.join(this.root, clientPath);
-      return nodePath.resolve(nodePath.normalize(nodePath.join(resolvedPath)));
-    })();
+    // Unix separators normalize nicer on both unix and win platforms
+    const resolvedPath = path.replace(WIN_SEP_REGEX, '/');
+
+    // Join cwd with new path
+    const joinedPath = nodePath.isAbsolute(resolvedPath)
+      ? nodePath.normalize(resolvedPath)
+      : nodePath.join('/', this.cwd, resolvedPath);
+
+    // Create local filesystem path using the platform separator
+    const fsPath = nodePath.resolve(nodePath.join(this.root, joinedPath)
+      .replace(UNIX_SEP_REGEX, nodePath.sep)
+      .replace(WIN_SEP_REGEX, nodePath.sep));
+
+    // Create FTP client path using unix separator
+    const clientPath = joinedPath.replace(WIN_SEP_REGEX, '/');
 
     return {
       clientPath,
       fsPath
     };
diff --git a/test/fs.spec.js b/test/fs.spec.js
index 3501fe8..8a9c229 100644
--- a/test/fs.spec.js
+++ b/test/fs.spec.js
@@ -34,11 +34,11 @@ describe('FileSystem', function () {
     });
   });
 
   describe('#_resolvePath', function () {
     it('gets correct relative path', function () {
-      const result = fs._resolvePath();
+      const result = fs._resolvePath('.');
       expect(result).to.be.an('object');
       expect(result.clientPath).to.equal(
         nodePath.normalize('/file/1/2/3'));
       expect(result.fsPath).to.equal(
         nodePath.resolve('/tmp/ftp-srv/file/1/2/3'));
@@ -51,28 +51,55 @@ describe('FileSystem', function () {
         nodePath.normalize('/file/1/2'));
       expect(result.fsPath).to.equal(
         nodePath.resolve('/tmp/ftp-srv/file/1/2'));
     });
 
+    it('gets correct relative path', function () {
+      const result = fs._resolvePath('other');
+      expect(result).to.be.an('object');
+      expect(result.clientPath).to.equal(
+        nodePath.normalize('/file/1/2/3/other'));
+      expect(result.fsPath).to.equal(
+        nodePath.resolve('/tmp/ftp-srv/file/1/2/3/other'));
+    });
+
     it('gets correct absolute path', function () {
       const result = fs._resolvePath('/other');
       expect(result).to.be.an('object');
       expect(result.clientPath).to.equal(
         nodePath.normalize('/other'));
       expect(result.fsPath).to.equal(
         nodePath.resolve('/tmp/ftp-srv/other'));
     });
 
-    it('cannot escape root', function () {
+    it('cannot escape root - unix', function () {
       const result = fs._resolvePath('../../../../../../../../../../..');
       expect(result).to.be.an('object');
       expect(result.clientPath).to.equal(
         nodePath.normalize('/'));
       expect(result.fsPath).to.equal(
         nodePath.resolve('/tmp/ftp-srv'));
     });
 
+    it('cannot escape root - win', function () {
+      const result = fs._resolvePath('.\\..\\..\\..\\..\\..\\..\\');
+      expect(result).to.be.an('object');
+      expect(result.clientPath).to.equal(
+        nodePath.normalize('/'));
+      expect(result.fsPath).to.equal(
+        nodePath.resolve('/tmp/ftp-srv'));
+    });
+
+    it('cannot escape root - backslash prefix', function () {
+      const result = fs._resolvePath('\\/../../../../../../');
+      expect(result).to.be.an('object');
+      expect(result.clientPath).to.equal(
+        nodePath.normalize('/'));
+      expect(result.fsPath).to.equal(
+        nodePath.resolve('/tmp/ftp-srv'));
+    });
+
     it('resolves to file', function () {
       const result = fs._resolvePath('/cool/file.txt');
       expect(result).to.be.an('object');
       expect(result.clientPath).to.equal(
         nodePath.normalize('/cool/file.txt'));
diff --git a/test/start.js b/test/start.js
index 24898a5..ca452a4 100644
--- a/test/start.js
+++ b/test/start.js
@@ -7,18 +7,18 @@ const server = new FtpServer({
   url: 'ftp://127.0.0.1:8880',
   pasv_url: '192.168.1.1',
   pasv_min: 8881,
   greeting: ['Welcome', 'to', 'the', 'jungle!'],
   tls: {
-    key: fs.readFileSync(`${process.cwd()}/test/cert/server.key`),
-    cert: fs.readFileSync(`${process.cwd()}/test/cert/server.crt`),
-    ca: fs.readFileSync(`${process.cwd()}/test/cert/server.csr`)
+    key: fs.readFileSync(`${__dirname}/cert/server.key`),
+    cert: fs.readFileSync(`${__dirname}/cert/server.crt`),
+    ca: fs.readFileSync(`${__dirname}/cert/server.csr`)
   },
   file_format: 'ep',
   anonymous: 'sillyrabbit'
 });
 server.on('login', ({username, password}, resolve, reject) => {
   if (username === 'test' && password === 'test' || username === 'anonymous') {
-    resolve({root: require('os').homedir()});
+    resolve({root: __dirname});
   } else reject('Bad username or password');
 });
 server.listen();

Vulnerable code:     this.cwd = nodePath.normalize(cwd ? nodePath.join(nodePath.sep, cwd) : nodePath.sep);
    const clientPath = (() => {
      path = nodePath.normalize(path);
      if (nodePath.isAbsolute(path)) {
        return nodePath.join(path);
      } else {
        return nodePath.join(this.cwd, path);
      }
    })();

    const fsPath = (() => {
      const resolvedPath = nodePath.join(this.root, clientPath);
      return nodePath.resolve(nodePath.normalize(nodePath.join(resolvedPath)));
    })();
      const result = fs._resolvePath();
    it('cannot escape root', function () {
    key: fs.readFileSync(`${process.cwd()}/test/cert/server.key`),
    cert: fs.readFileSync(`${process.cwd()}/test/cert/server.crt`),
    ca: fs.readFileSync(`${process.cwd()}/test/cert/server.csr`)
    resolve({root: require('os').homedir()});
Fixed code: const UNIX_SEP_REGEX = /\//g;
const WIN_SEP_REGEX = /\\/g;

    this.cwd = nodePath.normalize((cwd || '/').replace(WIN_SEP_REGEX, '/'));
    // Unix separators normalize nicer on both unix and win platforms
    const resolvedPath = path.replace(WIN_SEP_REGEX, '/');

    // Join cwd with new path
    const joinedPath = nodePath.isAbsolute(resolvedPath)
      ? nodePath.normalize(resolvedPath)
      : nodePath.join('/', this.cwd, resolvedPath);

    // Create local filesystem path using the platform separator
    const fsPath = nodePath.resolve(nodePath.join(this.root, joinedPath)
      .replace(UNIX_SEP_REGEX, nodePath.sep)
      .replace(WIN_SEP_REGEX, nodePath.sep));

    // Create FTP client path using unix separator
    const clientPath = joinedPath.replace(WIN_SEP_REGEX, '/');
      const result = fs._resolvePath('.');
    it('gets correct relative path', function () {
      const result = fs._resolvePath('other');
      expect(result).to.be.an('object');
      expect(result.clientPath).to.equal(
        nodePath.normalize('/file/1/2/3/other'));
      expect(result.fsPath).to.equal(
        nodePath.resolve('/tmp/ftp-srv/file/1/2/3/other'));
    });

    it('cannot escape root - unix', function () {
    it('cannot escape root - win', function () {
      const result = fs._resolvePath('.\\..\\..\\..\\..\\..\\..\\');
      expect(result).to.be.an('object');
      expect(result.clientPath).to.equal(
        nodePath.normalize('/'));
      expect(result.fsPath).to.equal(
        nodePath.resolve('/tmp/ftp-srv'));
    });

    it('cannot escape root - backslash prefix', function () {
      const result = fs._resolvePath('\\/../../../../../../');
      expect(result).to.be.an('object');
      expect(result.clientPath).to.equal(
        nodePath.normalize('/'));
      expect(result.fsPath).to.equal(
        nodePath.resolve('/tmp/ftp-srv'));
    });

    key: fs.readFileSync(`${__dirname}/cert/server.key`),
    cert: fs.readFileSync(`${__dirname}/cert/server.crt`),
    ca: fs.readFileSync(`${__dirname}/cert/server.csr`)
    resolve({root: __dirname});