PASV: make sure that the passive IP is valid (#236)

PASV: make sure that the passive IP is valid

an invalid IPv4 address will cause PASV handling crashes.

Fixes #235

transfer_pasv.go

@@ -37,6 +37,14 @@ type passiveTransferHandler struct {
 	logger      log.Logger       // Logger
 }
 
+type ipValidationError struct {
+	error string
+}
+
+func (e *ipValidationError) Error() string {
+	return e.error
+}
+
 func (c *clientHandler) getCurrentIP() ([]string, error) {
 	// Provide our external IP address so the ftp client can connect back to us
 	ip := c.server.settings.PublicHost
@@ -56,7 +64,17 @@ func (c *clientHandler) getCurrentIP() ([]string, error) {
 		}
 	}
 
-	return strings.Split(ip, "."), nil
+	parsedIP := net.ParseIP(ip)
+	if parsedIP == nil {
+		return nil, &ipValidationError{error: fmt.Sprintf("invalid passive IP %#v", ip)}
+	}
+
+	parsedIP = parsedIP.To4()
+	if parsedIP == nil {
+		return nil, &ipValidationError{error: fmt.Sprintf("invalid IPv4 passive IP %#v", ip)}
+	}
+
+	return strings.Split(parsedIP.String(), "."), nil
 }
 
 // ErrNoAvailableListeningPort is returned when no port could be found to accept incoming connection

transfer_test.go

@@ -822,3 +822,41 @@ func TestASCIITransfersInvalidFiles(t *testing.T) {
 	remoteHash := ftpDownloadAndHashWithRawConnection(t, raw, "file.bin")
 	require.Equal(t, localHash, remoteHash)
 }
+
+func TestPASVInvalidPublicHost(t *testing.T) {
+	s := NewTestServer(t, true)
+	s.settings.PublicHost = "not an IP"
+
+	conf := goftp.Config{
+		User:     authUser,
+		Password: authPass,
+	}
+
+	c, err := goftp.DialConfig(conf, s.Addr())
+	require.NoError(t, err, "Couldn't connect")
+
+	defer func() { require.NoError(t, c.Close()) }()
+
+	raw, err := c.OpenRawConn()
+	require.NoError(t, err, "Couldn't open raw connection")
+
+	rc, resp, err := raw.SendCommand("PASV")
+	require.NoError(t, err)
+	require.Equal(t, StatusServiceNotAvailable, rc)
+	require.Contains(t, resp, "invalid passive IP")
+
+	s.settings.PublicHost = "::1"
+	rc, resp, err = raw.SendCommand("PASV")
+	require.NoError(t, err)
+	require.Equal(t, StatusServiceNotAvailable, rc)
+	require.Contains(t, resp, "invalid IPv4 passive IP")
+
+	s.settings.PublicHost = ""
+	s.settings.PublicIPResolver = func(cc ClientContext) (string, error) {
+		return "127.0.0", nil
+	}
+	rc, resp, err = raw.SendCommand("PASV")
+	require.NoError(t, err)
+	require.Equal(t, StatusServiceNotAvailable, rc)
+	require.Contains(t, resp, "invalid passive IP")
+}