* ftpcmd.y: Don't dereference a NULL pointer returned from

	ftpglob().

	* ftpd.c: Be more paranoid about return values from ftpglob().
	Police uses of sprintf().  Account for expansion in
	radix_encode().

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@13193 dc483132-0cff-0310-8789-dd5450dbe970

3 files changed, 143 insertions, 20 deletions

diff --git a/src/appl/gssftp/ftpd/ChangeLog b/src/appl/gssftp/ftpd/ChangeLog
index 8ea2c7222..4da4317da 100644
--- a/src/appl/gssftp/ftpd/ChangeLog
+++ b/src/appl/gssftp/ftpd/ChangeLog
@@ -1,3 +1,12 @@
+2001-04-25  Tom Yu  <tlyu@mit.edu>
+
+	* ftpcmd.y: Don't dereference a NULL pointer returned from
+	ftpglob().
+
+	* ftpd.c: Be more paranoid about return values from ftpglob().
+	Police uses of sprintf().  Account for expansion in
+	radix_encode().
+
 2001-03-07  Ken Raeburn  <raeburn@mit.edu>
 
 	* ftpd.c (strerror): Only define if not HAVE_STRERROR.
diff --git a/src/appl/gssftp/ftpd/ftpcmd.y b/src/appl/gssftp/ftpd/ftpcmd.y
index 197815a6f..1c5ac1e0c 100644
--- a/src/appl/gssftp/ftpd/ftpcmd.y
+++ b/src/appl/gssftp/ftpd/ftpcmd.y
@@ -805,11 +805,15 @@ pathname:	pathstring
 		 * This is a valid reply in some cases but not in others.
 		 */
 		if (logged_in && $1 && strncmp((char *) $1, "~", 1) == 0) {
-			*(char **)&($$) = *ftpglob((char *) $1);
-			if (globerr != NULL) {
+			char **vv;
+
+			vv = ftpglob((char *) $1);
+			if (vv == NULL || globerr != NULL) {
 				reply(550, globerr);
 				$$ = NULL;
-			}
+			} else
+				$$ = *vv;
+
 			free((char *) $1);
 		} else
 			$$ = $1;
diff --git a/src/appl/gssftp/ftpd/ftpd.c b/src/appl/gssftp/ftpd/ftpd.c
index 0cd613e53..717fdd09a 100644
--- a/src/appl/gssftp/ftpd/ftpd.c
+++ b/src/appl/gssftp/ftpd/ftpd.c
@@ -763,7 +763,17 @@ user(name)
 		int result;
 #ifdef GSSAPI
 		if (auth_type && strcmp(auth_type, "GSSAPI") == 0) {
+			int len;
+
 			authorized = ftpd_gss_userok(&client_name, name) == 0;
+			len = sizeof("GSSAPI user  is not authorized as "
+				     "; Password required.")
+				+ strlen(client_name.value)
+				+ strlen(name);
+			if (len >= sizeof(buf)) {
+				syslog(LOG_ERR, "user: username too long");
+				name = "[username too long]";
+			}
 			sprintf(buf, "GSSAPI user %s is%s authorized as %s",
 				client_name.value, authorized ? "" : " not",
 				name);
@@ -774,7 +784,19 @@ user(name)
 #endif /* GSSAPI */
 #ifdef KRB5_KRB4_COMPAT
 		if (auth_type && strcmp(auth_type, "KERBEROS_V4") == 0) {
+			int len;
+
 			authorized = kuserok(&kdata,name) == 0;
+			len = sizeof("Kerberos user .@ is not authorized as "
+				     "; Password required.")
+				+ strlen(kdata.pname)
+				+ strlen(kdata.pinst)
+				+ strlen(kdata.prealm)
+				+ strlen(name);
+			if (len >= sizeof(buf)) {
+				syslog(LOG_ERR, "user: username too long");
+				name = "[username too long]";
+			}
 			sprintf(buf, "Kerberos user %s%s%s@%s is%s authorized as %s",
 				kdata.pname, *kdata.pinst ? "." : "",
 				kdata.pinst, kdata.prealm,
@@ -1181,6 +1203,11 @@ retrieve(cmd, name)
 	} else {
 		char line[FTP_BUFSIZ];
 
+		if (strlen(cmd) + strlen(name) + 1 >= sizeof(line)) {
+			syslog(LOG_ERR, "retrieve: filename too long");
+			reply(501, "filename too long");
+			return;
+		}
 		(void) sprintf(line, cmd, name), name = line;
 		fin = ftpd_popen(line, "r"), closefunc = ftpd_pclose;
 		st.st_size = -1;
@@ -1419,6 +1446,10 @@ dataconn(name, size, mode)
 	return (file);
 }
 
+/*
+ * XXX callers need to limit total length of output string to
+ * FTP_BUFSIZ
+ */
 #ifdef STDARG
 secure_error(char *fmt, ...)
 #else
@@ -1618,13 +1649,19 @@ statfilecmd(filename)
 {
 	char line[FTP_BUFSIZ];
 	FILE *fin;
-	int c;
+	int c, n;
 	char str[FTP_BUFSIZ], *p;
 
+	if (strlen(filename) + sizeof("/bin/ls -lgA ")
+	    >= sizeof(line)) {
+		reply(501, "filename too long");
+		return;
+	}
 	(void) sprintf(line, "/bin/ls -lgA %s", filename);
 	fin = ftpd_popen(line, "r");
 	lreply(211, "status of %s:", filename);
 	p = str;
+	n = 0;
 	while ((c = getc(fin)) != EOF) {
 		if (c == '\n') {
 			if (ferror(stdout)){
@@ -1641,7 +1678,16 @@ statfilecmd(filename)
 			*p = '\0';
 			reply(0, "%s", str);
 			p = str;
-		} else	*p++ = c;
+			n = 0;
+		} else {
+			*p++ = c;
+			n++;
+			if (n >= sizeof(str)) {
+				reply(551, "output line too long");
+				(void) ftpd_pclose(fin);
+				return;
+			}
+		}
 	}
 	if (p != str) {
 		*p = '\0';
@@ -1725,6 +1771,10 @@ fatal(s)
 
 char cont_char = ' ';
 
+/*
+ * XXX callers need to limit total length of output string to
+ * FTP_BUFSIZ bytes for now.
+ */
 #ifdef STDARG
 reply(int n, char *fmt, ...)
 #else
@@ -1746,22 +1796,32 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5)
 #endif
 
 	if (auth_type) {
-		char in[FTP_BUFSIZ], out[FTP_BUFSIZ];
+		/*
+		 * Deal with expansion in mk_{safe,priv},
+		 * radix_encode, gss_seal, plus slop.
+		 */
+		char in[FTP_BUFSIZ*3/2], out[FTP_BUFSIZ*3/2];
 		int length, kerror;
 		if (n) sprintf(in, "%d%c", n, cont_char);
 		else in[0] = '\0';
 		strncat(in, buf, sizeof (in) - strlen(in) - 1);
 #ifdef KRB5_KRB4_COMPAT
 		if (strcmp(auth_type, "KERBEROS_V4") == 0) {
-			if ((length = clevel == PROT_P ?
-			     krb_mk_priv((unsigned char *)in,
-					 (unsigned char *)out,
-					 strlen(in), schedule, &kdata.session,
-					 &ctrl_addr, &his_addr)
-			     : krb_mk_safe((unsigned char *)in,
-					   (unsigned char *)out,
-					   strlen(in), &kdata.session,
-					   &ctrl_addr, &his_addr)) == -1) {
+			if (clevel == PROT_P)
+				length = krb_mk_priv((unsigned char *)in,
+						     (unsigned char *)out,
+						     strlen(in),
+						     schedule, &kdata.session,
+						     &ctrl_addr,
+						     &his_addr);
+			else
+				length = krb_mk_safe((unsigned char *)in,
+						     (unsigned char *)out,
+						     strlen(in),
+						     &kdata.session,
+						     &ctrl_addr,
+						     &his_addr);
+			if (length == -1) {
 				syslog(LOG_ERR,
 				       "krb_mk_%s failed for KERBEROS_V4",
 				       clevel == PROT_P ? "priv" : "safe");
@@ -1805,13 +1865,16 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5)
 		}
 #endif /* GSSAPI */
 		/* Other auth types go here ... */
-		if (kerror = radix_encode(out, in, &length, 0)) {
+		if (length >= sizeof(in) / 4 * 3) {
+			syslog(LOG_ERR, "input to radix_encode too long");
+			fputs(in, stdout);
+		} else if (kerror = radix_encode(out, in, &length, 0)) {
 			syslog(LOG_ERR, "Couldn't encode reply (%s)",
 					radix_error(kerror));
 			fputs(in,stdout);
 		} else
-		printf("%s%c%s", clevel == PROT_P ? "632" : "631",
-				 n ? cont_char : '-', in);
+			printf("%s%c%s", clevel == PROT_P ? "632" : "631",
+			       n ? cont_char : '-', in);
 	} else {
 		if (n) printf("%d%c", n, cont_char);
 		fputs(buf, stdout);
@@ -1824,6 +1887,10 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5)
 	}
 }
 
+/*
+ * XXX callers need to limit total length of output string to
+ * FTP_BUFSIZ
+ */
 #ifdef STDARG
 lreply(int n, char *fmt, ...)
 #else
@@ -1868,7 +1935,8 @@ yyerror(s)
 
 	if (cp = strchr(cbuf,'\n'))
 		*cp = '\0';
-	reply(500, "'%s': command not understood.", cbuf);
+	reply(500, "'%.*s': command not understood.",
+	      FTP_BUFSIZ - sizeof("'': command not understood."), cbuf);
 }
 
 delete_file(name)
@@ -2145,7 +2213,23 @@ perror_reply(code, string)
 	int code;
 	char *string;
 {
-	reply(code, "%s: %s.", string, strerror(errno));
+	char *err_string;
+	size_t extra_len;
+
+	err_string = strerror(errno);
+	if (err_string == NULL)
+		err_string = "(unknown error)";
+	extra_len = strlen(err_string) + sizeof("(truncated): .");
+
+	/*
+	 * XXX knows about FTP_BUFSIZ in reply()
+	 */
+	if (strlen(string) + extra_len > FTP_BUFSIZ) {
+		reply(code, "(truncated)%.*s: %s.",
+		      FTP_BUFSIZ - extra_len, string, err_string);
+	} else {
+		reply(code, "%s: %s.", string, err_string);
+	}
 }
 
 auth(type)
@@ -2227,6 +2311,10 @@ char *data;
 			secure_error("ADAT: krb_mk_safe failed");
 			return(0);
 		}
+		if (length >= (FTP_BUFSIZ - sizeof("ADAT=")) / 4 * 3) {
+			secure_error("ADAT: reply too long");
+			return(0);
+		}
 		if (kerror = radix_encode(out_buf, buf, &length, 0)) {
 			secure_error("Couldn't encode ADAT reply (%s)",
 				     radix_error(kerror));
@@ -2361,6 +2449,16 @@ char *data;
 		}
 
 		if (out_tok.length) {
+			if (out_tok.length >= ((FTP_BUFSIZ - sizeof("ADAT="))
+					       / 4 * 3)) {
+				secure_error("ADAT: reply too long");
+				syslog(LOG_ERR, "ADAT: reply too long");
+				(void) gss_release_cred(&stat_min, &server_creds);
+				if (ret_flags & GSS_C_DELEG_FLAG)
+					(void) gss_release_cred(&stat_min,
+								&deleg_creds);
+				return(0);
+			}
 			if (kerror = radix_encode(out_tok.value, gbuf, &out_tok.length, 0)) {
 				secure_error("Couldn't encode ADAT reply (%s)",
 					     radix_error(kerror));
@@ -2459,6 +2557,9 @@ static char *onefile[] = {
  *	n>=0 on success
  *	-1 on error
  *	-2 on security error
+ *
+ * XXX callers need to limit total length of output string to
+ * FTP_BUFSIZ
  */
 #ifdef STDARG
 secure_fprintf(FILE *stream, char *fmt, ...)
@@ -2576,6 +2677,15 @@ send_file_list(whichfiles)
 			    dir->d_name[2] == '\0')
 				continue;
 
+			if (strlen(dirname) + strlen(dir->d_name)
+			    + 1 /* slash */
+			    + 2	/* CRLF */
+			    + 1 > sizeof(nbuf)) {
+				syslog(LOG_ERR,
+				       "send_file_list: pathname too long");
+				ret = -2; /* XXX */
+				goto data_err;
+			}
 			sprintf(nbuf, "%s/%s", dirname, dir->d_name);
 
 			/*