Subject: lib/24318: Parser bugs in getttyent(), getttynam()
To: None <gnats-bugs@gnats.netbsd.org>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 02/05/2004 04:41:39
>Number:         24318
>Category:       lib
>Synopsis:       Parser bugs in getttyent(), getttynam()
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 05 04:42:01 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Christian Biere
>Release:        NetBSD 1.6ZH
>Organization:
>Environment:
System: NetBSD cyclonus 1.6ZH NetBSD 1.6ZH (STARSCREAM) #1: Thu Jan 29 00:44:45 CET 2004 root@cyclonus:/usr/src/sys/arch/i386/compile/STARSCREAM i386
>Description:

getttyent() truncates the last field in every line of /etc/ttys by one
character. This bug is assumed to present in all BSDs and at minimum
12 years old. The problem is caused by this blatant line of code:

	*--t = `\0';

in skip(). If skip() parses the last field of the current line, so that
the parameter ``p'' points to '\0' the for-loop will not be entered and
the line above will overwrite the last character of the previous field
resulting in truncation of one character.

The next bugs exist only in NetBSD (or young derivates):
tty.ty_class is never initialized. Thus, if the classe field is not present
in the current line it will contain some semi-random content.
When switching from fgets() to the superior fgetln() the comment field
was effectively rendered useless because fgetln() as used stripped the
comment from any line.

>How-To-Repeat:

Try login in a xterm and wonder why TERM is set to "networ" instead of
the expected "network".

>Fix:

Index: lib/libc/gen/getttyent.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/getttyent.c,v
retrieving revision 1.20
diff -u -r1.20 getttyent.c
--- lib/libc/gen/getttyent.c	2003/08/07 16:42:51	1.20
+++ lib/libc/gen/getttyent.c	2004/02/01 00:08:39
@@ -56,7 +56,7 @@
 __weak_alias(setttyent,_setttyent)
 #endif
 
-static char zapchar;
+static char *comment;
 static FILE *tf;
 static size_t lineno = 0;
 static char *skip __P((char *));
@@ -93,12 +93,13 @@
 		free(line);
 	for (;;) {
 		errno = 0;
-		line = fparseln(tf, &len, &lineno, NULL, FPARSELN_UNESCALL);
+		line = fparseln(tf, &len, &lineno, "\\\\\0", FPARSELN_UNESCALL);
 		if (line == NULL) {
 			if (errno != 0)
 				warn("gettyent");
 			return NULL;
 		}
+		
 		for (p = line; *p && isspace((unsigned char) *p); p++)
 			continue;
 		if (*p && *p != '#')
@@ -106,9 +107,10 @@
 		free(line);
 	}
 
-	zapchar = 0;
+	comment = NULL;
 	tty.ty_name = p;
 	p = skip(p);
+
 	if (!*(tty.ty_getty = p))
 		tty.ty_getty = tty.ty_type = NULL;
 	else {
@@ -120,6 +122,7 @@
 	}
 	tty.ty_status = 0;
 	tty.ty_window = NULL;
+	tty.ty_class = NULL;
 
 #define	scmp(e)	!strncmp(p, e, sizeof(e) - 1) && (isspace((unsigned char) p[sizeof(e) - 1]) || p[sizeof(e) - 1] == '\0')
 #define	vcmp(e)	!strncmp(p, e, sizeof(e) - 1) && p[sizeof(e) - 1] == '='
@@ -148,13 +151,11 @@
 			warnx("gettyent: %s, %lu: unknown option `%s'",
 			    _PATH_TTYS, (unsigned long)lineno, p);
 	}
+
+	if (comment || (comment = strchr(p, '#')))
+		p = comment;
 
-	if (zapchar == '#' || *p == '#')
-		while ((c = *++p) == ' ' || c == '\t')
-			;
-	tty.ty_comment = p;
-	if (*p == 0)
-		tty.ty_comment = 0;
+	tty.ty_comment = *p ? p : NULL;
 	if ((p = strchr(p, '\n')) != NULL)
 		*p = '\0';
 	return (&tty);
@@ -167,15 +168,15 @@
  * the next field.
  */
 static char *
-skip(p)
-	char *p;
+skip(s)
+	char *s;
 {
-	char *t;
+	char *t, *p;
 	int c, q;
 
 	_DIAGASSERT(p != NULL);
 
-	for (q = 0, t = p; (c = *p) != '\0'; p++) {
+	for (q = 0, t = p = s; (c = (u_char)*p) != '\0'; p++) {
 		if (c == '"') {
 			q ^= QUOTED;	/* obscure, but nice */
 			continue;
@@ -186,19 +187,26 @@
 		if (q == QUOTED)
 			continue;
 		if (c == '#') {
-			zapchar = c;
-			*p = 0;
+			*p = '\0';
+			comment = p + 1;
 			break;
 		}
-		if (c == '\t' || c == ' ' || c == '\n') {
-			zapchar = c;
+		if (isspace(c)) {
 			*p++ = 0;
-			while ((c = *p) == '\t' || c == ' ' || c == '\n')
+			while (isspace((u_char)*p))
 				p++;
+			if (*p == '#') {
+				*p++ = '\0';
+				while (isspace((u_char)*p))
+					p++;
+				comment = p;
+				p = strchr(p, '\0');
+			}
 			break;
 		}
 	}
-	*--t = '\0';
+	if (t > s && c != '\0')
+		*--t = '\0';
 	return (p);
 }
 
>Release-Note:
>Audit-Trail:
>Unformatted: