Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Jul 2002 19:33:29 +0400
From:      Yar Tikhiy <yar@freebsd.org>
To:        audit@freebsd.org
Subject:   Ftpd(8) reading config files
Message-ID:  <20020710193329.A8572@comp.chem.msu.su>

next in thread | raw e-mail | index | archive | help
Hi there,

Currently, our ftpd(8) suffers from:

a) poor handling of lines read from /etc/ftpusers and /etc/ftphosts,
   which are longer than the buffer passed to fgets(3).
   Of course, user names are short, and even comments will fit into
   the buffer in most cases, but it doesn't forgive the poor code.

b) wrong handling of the last line in the files which doesn't end with
   a newline (PR misc/21494).

Maxim, mikeh and I worked out the following remedy to the above
problems.  Could you review it?  Sorry, it looks overengineered,
but I can't see how it could be simpler.

-- 
Yar

Index: ftpd.c
===================================================================
RCS file: /home/ncvs/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.105
diff -u -w -r1.105 ftpd.c
--- ftpd.c	3 Jul 2002 00:12:00 -0000	1.105
+++ ftpd.c	9 Jul 2002 10:59:00 -0000
@@ -651,20 +651,24 @@
 static void
 inithosts(void)
 {
+	size_t len;
 	FILE *fp;
-	char *cp;
+	char *cp, *mp, *line;
+	char *hostname;
 	struct ftphost *hrp, *lhrp;
-	char line[1024];
 	struct addrinfo hints, *res, *ai;
 
 	/*
 	 * Fill in the default host information
 	 */
-	if (gethostname(line, sizeof(line)) < 0)
-		line[0] = '\0';
-	if ((hrp = malloc(sizeof(struct ftphost))) == NULL ||
-	    (hrp->hostname = strdup(line)) == NULL)
+	if ((hostname = malloc(MAXHOSTNAMELEN)) == NULL)
 		fatalerror("Ran out of memory.");
+	if (gethostname(hostname, MAXHOSTNAMELEN) < 0)
+		hostname[0] = '\0';
+	hostname[MAXHOSTNAMELEN - 1] = '\0';
+	if ((hrp = malloc(sizeof(struct ftphost))) == NULL)
+		fatalerror("Ran out of memory.");
+	hrp->hostname = hostname;
 	hrp->hostinfo = NULL;
 
 	memset(&hints, 0, sizeof(hints));
@@ -684,28 +688,33 @@
 		void *addr;
 		struct hostent *hp;
 
-		while (fgets(line, sizeof(line), fp) != NULL) {
+		while ((line = fgetln(fp, &len)) != NULL) {
 			int	i, hp_error;
 
-			if ((cp = strchr(line, '\n')) == NULL) {
-				/* ignore long lines */
-				while (fgets(line, sizeof(line), fp) != NULL &&
-					strchr(line, '\n') == NULL)
-					;
+			/* skip comments */
+			if (line[0] == '#')
 				continue;
+			if (line[len - 1] == '\n') {
+				line[len - 1] = '\0';
+				mp = NULL;
+			} else {
+				if ((mp = malloc(len + 1)) == NULL)
+					fatalerror("Ran out of memory.");
+				memcpy(mp, line, len);
+				mp[len] = '\0';
+				line = mp;
 			}
-			*cp = '\0';
 			cp = strtok(line, " \t");
-			/* skip comments and empty lines */
-			if (cp == NULL || line[0] == '#')
-				continue;
+			/* skip empty lines */
+			if (cp == NULL)
+				goto nextline;
 
 			hints.ai_flags = 0;
 			hints.ai_family = AF_UNSPEC;
 			hints.ai_flags = AI_PASSIVE;
 			error = getaddrinfo(cp, NULL, &hints, &res);
 			if (error != NULL)
-				continue;
+				goto nextline;
 			for (ai = res; ai != NULL && ai->ai_addr != NULL;
 			     ai = ai->ai_next) {
 
@@ -727,7 +736,7 @@
 			}
 			if (hrp == NULL) {
 				if ((hrp = malloc(sizeof(struct ftphost))) == NULL)
-					continue;
+					goto nextline;
 				/* defaults */
 				hrp->statfile = _PATH_FTPDSTATFILE;
 				hrp->welcome  = _PATH_FTPWELCOME;
@@ -759,7 +768,7 @@
 				if (hrp->hostinfo != NULL)
 					freeaddrinfo(hrp->hostinfo);
 				free(hrp);
-				continue;
+				goto nextline;
 				/* NOTREACHED */
 			}
 			if ((hp = getipnodebyaddr((char*)addr, addrsize,
@@ -804,6 +813,9 @@
 			/* XXX: re-initialization for getaddrinfo() loop */
 			cp = strtok(line, " \t");
 		      }
+nextline:
+			if (mp)
+				free(mp);
 		}
 		(void) fclose(fp);
 	}
@@ -1013,23 +1025,38 @@
 {
 	FILE *fd;
 	int found = 0;
-	char *p, line[BUFSIZ];
+	size_t len;
+	char *line, *mp, *p;
 
 	if ((fd = fopen(fname, "r")) != NULL) {
-		while (!found && fgets(line, sizeof(line), fd) != NULL)
-			if ((p = strchr(line, '\n')) != NULL) {
-				*p = '\0';
+		while (!found && (line = fgetln(fd, &len)) != NULL) {
+			/* skip comments */
 				if (line[0] == '#')
 					continue;
+			if (line[len - 1] == '\n') {
+				line[len - 1] = '\0';
+				mp = NULL;
+			} else {
+				if ((mp = malloc(len + 1)) == NULL)
+					fatalerror("Ran out of memory.");
+				memcpy(mp, line, len);
+				mp[len] = '\0';
+				line = mp;
+			}
+			/* avoid possible leading and trailing whitespace */
+			p = strtok(line, " \t");
+			/* skip empty lines */
+			if (p == NULL)
+				goto nextline;
 				/*
 				 * if first chr is '@', check group membership
 				 */
-				if (line[0] == '@') {
+			if (p[0] == '@') {
 					int i = 0;
 					struct group *grp;
 
-					if ((grp = getgrnam(line+1)) == NULL)
-						continue;
+				if ((grp = getgrnam(p+1)) == NULL)
+					goto nextline;
 					/*
 					 * Check user's default group
 					 */
@@ -1047,7 +1074,10 @@
 				 * Otherwise, just check for username match
 				 */
 				else
-					found = strcmp(line, name) == 0;
+				found = strcmp(p, name) == 0;
+nextline:
+			if (mp)
+				free(mp);
 			}
 		(void) fclose(fd);
 	}

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020710193329.A8572>