From 124c471a2bf430f6958351d56af618a21e55d093 Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Fri, 16 Apr 2021 15:08:10 +0200
Subject: [PATCH] FTPD: make sure that the passive ip, if provided, is valid

The server will refuse to start if the provided passive ip is not a
valid IPv4 address.

Fixes #376
---
 docs/full-configuration.md |  2 +-
 ftpd/ftpd.go               | 16 ++++++++++++++++
 ftpd/ftpd_test.go          |  5 +++++
 ftpd/internal_test.go      | 21 +++++++++++++++++++++
 ftpd/server.go             |  3 +++
 5 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/docs/full-configuration.md b/docs/full-configuration.md
index 6216c749..c5620ade 100644
--- a/docs/full-configuration.md
+++ b/docs/full-configuration.md
@@ -109,7 +109,7 @@ The configuration file contains the following sections:
     - `address`, string. Leave blank to listen on all available network interfaces. Default: "".
     - `apply_proxy_config`, boolean. If enabled the common proxy configuration, if any, will be applied. Default `true`.
     - `tls_mode`, integer. 0 means accept both cleartext and encrypted sessions. 1 means TLS is required for both control and data connection. 2 means implicit TLS. Do not enable this blindly, please check that a proper TLS config is in place if you set `tls_mode` is different from 0.
-    - `force_passive_ip`, ip address. External IP address to expose for passive connections. Leavy empty to autodetect. Defaut: "".
+    - `force_passive_ip`, ip address. External IP address to expose for passive connections. Leavy empty to autodetect. If not empty, it must be a valid IPv4 address. Defaut: "".
     - `client_auth_type`, integer. Set to `1` to require a client certificate and verify it. Set to `2` to request a client certificate during the TLS handshake and verify it if given, in this mode the client is allowed not to send a certificate. At least one certification authority must be defined in order to verify client certificates. If no certification authority is defined, this setting is ignored. Default: 0.
     - `tls_cipher_suites`, list of strings. List of supported cipher suites for TLS version 1.2. If empty, a default list of secure cipher suites is used, with a preference order based on hardware performance. Note that TLS 1.3 ciphersuites are not configurable. The supported ciphersuites names are defined [here](https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L52). Any invalid name will be silently ignored. The order matters, the ciphers listed first will be the preferred ones. Default: empty.
   - `bind_port`, integer. Deprecated, please use `bindings`
diff --git a/ftpd/ftpd.go b/ftpd/ftpd.go
index e6925c2c..eb8970a8 100644
--- a/ftpd/ftpd.go
+++ b/ftpd/ftpd.go
@@ -3,6 +3,7 @@ package ftpd
 
 import (
 	"fmt"
+	"net"
 	"path/filepath"
 
 	ftpserver "github.com/fclairamb/ftpserverlib"
@@ -74,6 +75,21 @@ func (b *Binding) IsValid() bool {
 	return b.Port > 0
 }
 
+func (b *Binding) checkPassiveIP() error {
+	if b.ForcePassiveIP != "" {
+		ip := net.ParseIP(b.ForcePassiveIP)
+		if ip == nil {
+			return fmt.Errorf("the provided passive IP %#v is not valid", b.ForcePassiveIP)
+		}
+		ip = ip.To4()
+		if ip == nil {
+			return fmt.Errorf("the provided passive IP %#v is not a valid IPv4 address", b.ForcePassiveIP)
+		}
+		b.ForcePassiveIP = ip.String()
+	}
+	return nil
+}
+
 // HasProxy returns true if the proxy protocol is active for this binding
 func (b *Binding) HasProxy() bool {
 	return b.ApplyProxyConfig && common.Config.ProxyProtocol > 0
diff --git a/ftpd/ftpd_test.go b/ftpd/ftpd_test.go
index 90f9bb90..fd6ebd29 100644
--- a/ftpd/ftpd_test.go
+++ b/ftpd/ftpd_test.go
@@ -454,6 +454,11 @@ func TestInitializationFailure(t *testing.T) {
 
 	ftpdConf.CACertificates = []string{caCrtPath}
 	ftpdConf.CARevocationLists = []string{caCRLPath}
+	ftpdConf.Bindings[1].ForcePassiveIP = "127001"
+	err = ftpdConf.Initialize(configDir)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "the provided passive IP \"127001\" is not valid")
+	ftpdConf.Bindings[1].ForcePassiveIP = ""
 	err = ftpdConf.Initialize(configDir)
 	require.Error(t, err)
 }
diff --git a/ftpd/internal_test.go b/ftpd/internal_test.go
index 3c526418..9a8a3d1a 100644
--- a/ftpd/internal_test.go
+++ b/ftpd/internal_test.go
@@ -387,6 +387,27 @@ func TestInitialization(t *testing.T) {
 	_, err = server.GetSettings()
 	assert.Error(t, err)
 
+	binding = Binding{
+		Port:           2121,
+		ForcePassiveIP: "192.168.1",
+	}
+	server = NewServer(c, configDir, binding, 0)
+	_, err = server.GetSettings()
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "is not valid")
+	}
+
+	binding.ForcePassiveIP = "::ffff:192.168.89.9"
+	err = binding.checkPassiveIP()
+	assert.NoError(t, err)
+	assert.Equal(t, "192.168.89.9", binding.ForcePassiveIP)
+
+	binding.ForcePassiveIP = "::1"
+	err = binding.checkPassiveIP()
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "is not a valid IPv4 address")
+	}
+
 	err = ReloadCertificateMgr()
 	assert.NoError(t, err)
 
diff --git a/ftpd/server.go b/ftpd/server.go
index 23c26eae..82ba53e4 100644
--- a/ftpd/server.go
+++ b/ftpd/server.go
@@ -83,6 +83,9 @@ func (s *Server) cleanTLSConnVerification(id uint32) {
 
 // GetSettings returns FTP server settings
 func (s *Server) GetSettings() (*ftpserver.Settings, error) {
+	if err := s.binding.checkPassiveIP(); err != nil {
+		return nil, err
+	}
 	var portRange *ftpserver.PortRange
 	if s.config.PassivePortRange.Start > 0 && s.config.PassivePortRange.End > s.config.PassivePortRange.Start {
 		portRange = &ftpserver.PortRange{