CVE-2020-15152: Server-Side Request Forgery (SSRF) (CWE-918)
value: ftp-srv is an npm package which is a modern and extensible FTP server designed to be simple yet configurable. In ftp-srv before versions 2.19.6, 3.1.2, and 4.3.4 are vulnerable to Server-Side Request Forgery. The PORT command allows arbitrary IPs which can be used to cause the server to make a connection elsewhere. A possible workaround is blocking the PORT through the configuration. This issue is fixed in version2 2.19.6, 3.1.2, and 4.3.4. More information can be found on the linked advisory.
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 9.1
Published: 2020-08-17T22:15Z
Repository: https://github.com/autovance/ftp-srv
Language: JavaScript
Files changed: src/commands/registration/eprt.js, src/commands/registration/epsv.js, src/commands/registration/pasv.js, src/commands/registration/port.js, src/connector/active.js, src/connector/base.js, test/commands/registration/eprt.spec.js, test/commands/registration/epsv.spec.js, test/commands/registration/opts.spec.js, test/connector/active.spec.js
Commit date: 2020-08-17 13:15:06 -0600
Commit message: fix: disallow PORT connections to alternate hosts

Ensure the data socket that the server connects to from the PORT command is the same IP as the current command socket.

* fix: add error handling to additional connection commands

Diff: diff --git a/src/commands/registration/eprt.js b/src/commands/registration/eprt.js
index 4ced2ef..2607a2a 100644
--- a/src/commands/registration/eprt.js
+++ b/src/commands/registration/eprt.js
@@ -6,17 +6,21 @@ const FAMILY = {
   2: 6
 };
 
 module.exports = {
   directive: 'EPRT',
-  handler: function ({command} = {}) {
+  handler: function ({log, command} = {}) {
     const [, protocol, ip, port] = _.chain(command).get('arg', '').split('|').value();
     const family = FAMILY[protocol];
     if (!family) return this.reply(504, 'Unknown network protocol');
 
     this.connector = new ActiveConnector(this);
     return this.connector.setupConnection(ip, port, family)
-    .then(() => this.reply(200));
+    .then(() => this.reply(200))
+    .catch((err) => {
+      log.error(err);
+      return this.reply(err.code || 425, err.message);
+    });
   },
   syntax: '{{cmd}} |<protocol>|<address>|<port>|',
   description: 'Specifies an address and port to which the server should connect'
 };
diff --git a/src/commands/registration/epsv.js b/src/commands/registration/epsv.js
index 7e524b8..f60fffd 100644
--- a/src/commands/registration/epsv.js
+++ b/src/commands/registration/epsv.js
@@ -1,16 +1,20 @@
 const PassiveConnector = require('../../connector/passive');
 
 module.exports = {
   directive: 'EPSV',
-  handler: function () {
+  handler: function ({log}) {
     this.connector = new PassiveConnector(this);
     return this.connector.setupServer()
     .then((server) => {
       const {port} = server.address();
 
       return this.reply(229, `EPSV OK (|||${port}|)`);
+    })
+    .catch((err) => {
+      log.error(err);
+      return this.reply(err.code || 425, err.message);
     });
   },
   syntax: '{{cmd}} [<protocol>]',
   description: 'Initiate passive mode'
 };
diff --git a/src/commands/registration/pasv.js b/src/commands/registration/pasv.js
index 73ee497..c03e1d5 100644
--- a/src/commands/registration/pasv.js
+++ b/src/commands/registration/pasv.js
@@ -23,11 +23,11 @@ module.exports = {
 
       return this.reply(227, `PASV OK (${host},${portByte1},${portByte2})`);
     })
     .catch((err) => {
       log.error(err);
-      return this.reply(425);
+      return this.reply(err.code || 425, err.message);
     });
   },
   syntax: '{{cmd}}',
   description: 'Initiate passive mode'
 };
diff --git a/src/commands/registration/port.js b/src/commands/registration/port.js
index 248f185..a183a74 100644
--- a/src/commands/registration/port.js
+++ b/src/commands/registration/port.js
@@ -15,11 +15,11 @@ module.exports = {
 
     return this.connector.setupConnection(ip, port)
     .then(() => this.reply(200))
     .catch((err) => {
       log.error(err);
-      return this.reply(425);
+      return this.reply(err.code || 425, err.message);
     });
   },
   syntax: '{{cmd}} <x>,<x>,<x>,<x>,<y>,<y>',
   description: 'Specifies an address and port to which the server should connect'
 };
diff --git a/src/connector/active.js b/src/connector/active.js
index 93726cf..b9c3d86 100644
--- a/src/connector/active.js
+++ b/src/connector/active.js
@@ -1,9 +1,11 @@
 const {Socket} = require('net');
 const tls = require('tls');
+const ip = require('ip');
 const Promise = require('bluebird');
 const Connector = require('./base');
+const {SocketError} = require('../errors');
 
 class Active extends Connector {
   constructor(connection) {
     super(connection);
     this.type = 'active';
@@ -25,10 +27,14 @@ class Active extends Connector {
     const closeExistingServer = () => Promise.resolve(
       this.dataSocket ? this.dataSocket.destroy() : undefined);
 
     return closeExistingServer()
     .then(() => {
+      if (!ip.isEqual(this.connection.commandSocket.remoteAddress, host)) {
+        throw new SocketError('The given address is not yours', 500);
+      }
+
       this.dataSocket = new Socket();
       this.dataSocket.on('error', (err) => this.server && this.server.emit('client-error', {connection: this.connection, context: 'dataSocket', error: err}));
       this.dataSocket.connect({host, port, family}, () => {
         this.dataSocket.pause();
 
diff --git a/src/connector/base.js b/src/connector/base.js
index eed9937..33707a3 100644
--- a/src/connector/base.js
+++ b/src/connector/base.js
@@ -27,11 +27,11 @@ class Connector {
   }
 
   closeSocket() {
     if (this.dataSocket) {
       const socket = this.dataSocket;
-      this.dataSocket.end(() => socket.destroy());
+      this.dataSocket.end(() => socket && socket.destroy());
       this.dataSocket = null;
     }
   }
 
   closeServer() {
diff --git a/test/commands/registration/eprt.spec.js b/test/commands/registration/eprt.spec.js
index 6fc119f..4c3990e 100644
--- a/test/commands/registration/eprt.spec.js
+++ b/test/commands/registration/eprt.spec.js
@@ -21,11 +21,11 @@ describe(CMD, function () {
   afterEach(() => {
     sandbox.restore();
   });
 
   it('// unsuccessful | no argument', () => {
-    return cmdFn()
+    return cmdFn({})
     .then(() => {
       expect(mockClient.reply.args[0][0]).to.equal(504);
     });
   });
 
diff --git a/test/commands/registration/epsv.spec.js b/test/commands/registration/epsv.spec.js
index 00c4561..5ab7a5a 100644
--- a/test/commands/registration/epsv.spec.js
+++ b/test/commands/registration/epsv.spec.js
@@ -23,11 +23,11 @@ describe(CMD, function () {
   afterEach(() => {
     sandbox.restore();
   });
 
   it('// successful IPv4', () => {
-    return cmdFn()
+    return cmdFn({})
     .then(() => {
       const [code, message] = mockClient.reply.args[0];
       expect(code).to.equal(229);
       expect(message).to.equal('EPSV OK (|||12345|)');
     });
diff --git a/test/commands/registration/opts.spec.js b/test/commands/registration/opts.spec.js
index 99171e3..d95ada4 100644
--- a/test/commands/registration/opts.spec.js
+++ b/test/commands/registration/opts.spec.js
@@ -27,11 +27,11 @@ describe(CMD, function () {
   });
 
   it('BAD // unsuccessful', () => {
     return cmdFn({command: {arg: 'BAD', directive: CMD}})
     .then(() => {
-      expect(mockClient.reply.args[0][0]).to.equal(500);
+      expect(mockClient.reply.args[0][0]).to.equal(501);
     });
   });
 
   it('UTF8 BAD // unsuccessful', () => {
     return cmdFn({command: {arg: 'UTF8 BAD', directive: CMD}})
diff --git a/test/connector/active.spec.js b/test/connector/active.spec.js
index 827505f..dc36b2f 100644
--- a/test/connector/active.spec.js
+++ b/test/connector/active.spec.js
@@ -11,47 +11,66 @@ const {getNextPortFactory} = require('../../src/helpers/find-port');
 describe('Connector - Active //', function () {
   const host = '127.0.0.1';
   let getNextPort = getNextPortFactory(host, 1024);
   let PORT;
   let active;
-  let mockConnection = {};
+  let mockConnection = {
+    commandSocket: {
+      remoteAddress: '::ffff:127.0.0.1'
+    }
+  };
   let sandbox;
   let server;
 
-  before(() => {
-    active = new ActiveConnector(mockConnection);
-  });
   beforeEach((done) => {
+    active = new ActiveConnector(mockConnection);
     sandbox = sinon.sandbox.create().usingPromise(Promise);
 
     getNextPort()
     .then((port) => {
       PORT = port;
       server = net.createServer()
       .on('connection', (socket) => socket.destroy())
       .listen(PORT, () => done());
     });
   });
-  afterEach((done) => {
+
+  afterEach(() => {
     sandbox.restore();
-    server.close(done);
+    server.close();
+    active.end();
+
   });
 
   it('sets up a connection', function () {
     return active.setupConnection(host, PORT)
     .then(() => {
       expect(active.dataSocket).to.exist;
     });
   });
 
-  it('destroys existing connection, then sets up a connection', function () {
-    const destroyFnSpy = sandbox.spy(active.dataSocket, 'destroy');
+  it('rejects alternative host', function () {
+    return active.setupConnection('123.45.67.89', PORT)
+    .catch((err) => {
+      expect(err.code).to.equal(500);
+      expect(err.message).to.equal('The given address is not yours');
+    })
+    .finally(() => {
+      expect(active.dataSocket).not.to.exist;
+    });
+  });
 
+  it('destroys existing connection, then sets up a connection', function () {
     return active.setupConnection(host, PORT)
     .then(() => {
-      expect(destroyFnSpy.callCount).to.equal(1);
-      expect(active.dataSocket).to.exist;
+      const destroyFnSpy = sandbox.spy(active.dataSocket, 'destroy');
+
+      return active.setupConnection(host, PORT)
+      .then(() => {
+        expect(destroyFnSpy.callCount).to.equal(1);
+        expect(active.dataSocket).to.exist;
+      });
     });
   });
 
   it('waits for connection', function () {
     return active.setupConnection(host, PORT)

Vulnerable code:   handler: function ({command} = {}) {
    .then(() => this.reply(200));
  handler: function () {
      return this.reply(425);
      return this.reply(425);
      this.dataSocket.end(() => socket.destroy());
    return cmdFn()
    return cmdFn()
      expect(mockClient.reply.args[0][0]).to.equal(500);
  let mockConnection = {};
  before(() => {
    active = new ActiveConnector(mockConnection);
  });
  afterEach((done) => {
    server.close(done);
  it('destroys existing connection, then sets up a connection', function () {
    const destroyFnSpy = sandbox.spy(active.dataSocket, 'destroy');
      expect(destroyFnSpy.callCount).to.equal(1);
      expect(active.dataSocket).to.exist;
Fixed code:   handler: function ({log, command} = {}) {
    .then(() => this.reply(200))
    .catch((err) => {
      log.error(err);
      return this.reply(err.code || 425, err.message);
    });
  handler: function ({log}) {
    })
    .catch((err) => {
      log.error(err);
      return this.reply(err.code || 425, err.message);
      return this.reply(err.code || 425, err.message);
      return this.reply(err.code || 425, err.message);
const ip = require('ip');
const {SocketError} = require('../errors');
      if (!ip.isEqual(this.connection.commandSocket.remoteAddress, host)) {
        throw new SocketError('The given address is not yours', 500);
      }

      this.dataSocket.end(() => socket && socket.destroy());
    return cmdFn({})
    return cmdFn({})
      expect(mockClient.reply.args[0][0]).to.equal(501);
  let mockConnection = {
    commandSocket: {
      remoteAddress: '::ffff:127.0.0.1'
    }
  };
    active = new ActiveConnector(mockConnection);

  afterEach(() => {
    server.close();
    active.end();

  it('rejects alternative host', function () {
    return active.setupConnection('123.45.67.89', PORT)
    .catch((err) => {
      expect(err.code).to.equal(500);
      expect(err.message).to.equal('The given address is not yours');
    })
    .finally(() => {
      expect(active.dataSocket).not.to.exist;
    });
  });
  it('destroys existing connection, then sets up a connection', function () {
      const destroyFnSpy = sandbox.spy(active.dataSocket, 'destroy');

      return active.setupConnection(host, PORT)
      .then(() => {
        expect(destroyFnSpy.callCount).to.equal(1);
        expect(active.dataSocket).to.exist;
      });