>Number:         12558
>Category:       bin
>Synopsis:       ftpd(8) sometimes make PORT command error.
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Apr 05 07:44:00 PDT 2001
>Closed-Date:
>Last-Modified:
>Originator:     Takahiro Kambe
>Release:        NetBSD-current 2001/4/1
>Organization:
	
>Environment:
	
System: NetBSD edge.sky.yamashina.kyoto.jp 1.5T NetBSD 1.5T (CF-M33) #115: Sun Apr 1 16:23:27 JST 2001 taca@edge.sky.yamashina.kyoto.jp:/usr/src/sys/arch/i386/compile/CF-M33 i386
Architecture: i386
Machine: i386
>Description:
	ftpd(8) sometimes make PORT command error, even if address and port
	have no problem.
>How-To-Repeat:
	With a line bellow in /etc/ftpd.conf:

	checkportcmd all

	Try connect localhost, and repeat chdir and dir.

>Fix:
	At line 1800 in ftpcmd.y, checkport() try to compare IPv4 address,
	but it really compare length of sockaddr.

	The patch bellow also contains logging why checkport() made it error.

--- ftpcmd.y.orig	Thu Apr  5 23:25:59 2001
+++ ftpcmd.y	Thu Apr  5 23:26:38 2001
@@ -1782,24 +1782,53 @@ port_check(const char *cmd, int family)
 	}
 
 	if (family != -1 && his_addr.su_family != family) {
+		syslog(LOG_DEBUG,
+			"port_check: %d is different family from me %d",
+			his_addr.su_family, family);
  port_check_fail:
 		reply(500, "Illegal %s command rejected", cmd);
 		return;
 	}
 
-	if (data_dest.su_family != his_addr.su_family)
+	if (data_dest.su_family != his_addr.su_family) {
+		syslog(LOG_DEBUG,
+			"port_check: data %d is different family from his %d",
+			data_dest.su_family, his_addr.su_family);
 		goto port_check_fail;
+	}
 
 			/* be paranoid, if told so */
 	if (CURCLASS_FLAGS_ISSET(checkportcmd)) {
-		if ((ntohs(data_dest.su_port) < IPPORT_RESERVED) ||
-		    (data_dest.su_len != his_addr.su_len))
+		if ((ntohs(data_dest.su_port) < IPPORT_RESERVED)) {
+			syslog(LOG_DEBUG,
+				"port_check: request reserved port",
+				ntohs(data_dest.su_port));
+			goto port_check_fail;
+		}
+		if (data_dest.su_len != his_addr.su_len) {
+			syslog(LOG_DEBUG,
+				"port_check: su_len mis-match %d and %d",
+				data_dest.su_len, his_addr.su_len);
 			goto port_check_fail;
+		}
+
 		switch (data_dest.su_family) {
 		case AF_INET:
 			if (memcmp(&data_dest.su_addr, &his_addr.su_addr,
-			    data_dest.su_len) != 0)
+			    sizeof(his_addr.su_addr)) != 0) {
+				char *s, *t;
+
+				s = inet_ntoa(data_dest.su_addr);
+				t = inet_ntoa(his_addr.su_addr);
+				syslog(LOG_DEBUG,
+					"port_check: address mismatch %s and %s",
+					(s)? s: "(unknown)",
+					(t)? t: "(unknown)");
+				syslog(LOG_DEBUG,
+					"port_check: length was %d",
+					data_dest.su_len);
 				goto port_check_fail;
+			}
 			break;
 #ifdef INET6
 		case AF_INET6:
@@ -1811,6 +1840,9 @@ port_check(const char *cmd, int family)
 			break;
 #endif
 		default:
+			syslog(LOG_DEBUG,
+				"port_check: unknown family %d",
+				data_dest.su_family);
 			goto port_check_fail;
 		}
 	}
>Release-Note:
>Audit-Trail:
>Unformatted: