Subject: bin/19071: write(1) prints uninitialized errnos under odd conditions
To: None <gnats-bugs@gnats.netbsd.org>
From: None <dholland@eecs.harvard.edu>
List: netbsd-bugs
Date: 11/16/2002 00:57:27
>Number:         19071
>Category:       bin
>Synopsis:       write prints uninitialized errnos under certain conditions
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Nov 15 21:58:00 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>Release:        NetBSD 1.6H (-20020920)
>Organization:
   - David A. Holland / dholland@eecs.harvard.edu
>Environment:
System: NetBSD alicante 1.6H NetBSD 1.6H (ALICANTE) #4: Fri Sep 20 14:00:10 EDT 2002 dholland@alicante:/usr/src/sys/arch/i386/compile/ALICANTE i386
Architecture: i386
Machine: i386
>Description:

	If you don't own your tty, write prints an uninitialized errno.

>How-To-Repeat:

	Log in. su to root. su to some other user, not the same as the
	first, such as daemon or news or whatever.

	Use write(1) to try to write to yourself, or to anyone else.

	Write prints "/dev/yourtty: Unknown error (error 0)".

>Fix:

	It turns out that in write.c the function term_chk() is used
	to perform various tty sanity checks, both on target ttys for
	writing and on one's own tty.

	In the current version, this function is expected to return -1
	and set errno when it fails. In the case where the tty
	ownership is wrong, however, it fails to set errno.

	In preparing the following patch, rather than just setting
	errno in this case, I've changed the function so instead of
	setting errno, it prints its own error messages (and exits) if
	requested. This is because there isn't any errno that won't
	(IMO) result in a really misleading error message.

	Note that the call sites that request term_chk to print an
	error and exit still check it for error; this is not an
	oversight but defensive coding. :-)

	The patch is against the latest version of write.c (1.21).

Index: write.c
===================================================================
RCS file: /cvsroot/basesrc/usr.bin/write/write.c,v
retrieving revision 1.21
diff -u -r1.21 write.c
--- write.c	2002/08/16 20:21:49	1.21
+++ write.c	2002/11/16 05:13:30
@@ -72,7 +72,7 @@
 void do_write(int, const char *, const uid_t);
 void wr_fputs(char *);
 int search_utmp(char *, char *, uid_t);
-int term_chk(uid_t, const char *, int *, time_t *, int);
+int term_chk(uid_t, const char *, int *, time_t *, int, int);
 int utmp_chk(const char *, const char *);
 int main(int, char **);
 
@@ -106,8 +106,8 @@
 		errx(1, "can't find your tty's name");
 	if ((cp = strrchr(mytty, '/')) != NULL)
 		mytty = cp + 1;
-	if (term_chk(myuid, mytty, &msgsok, &atime, 1) == -1)
-		err(1, "%s%s", _PATH_DEV, mytty);
+	if (term_chk(myuid, mytty, &msgsok, &atime, 1, 1) == -1)
+		errx(1, "%s%s: Cannot access own tty", _PATH_DEV, mytty);
 	if (!msgsok) {
 		(void)fprintf(stderr,
 		    "warning: you have write permission turned off; "
@@ -127,9 +127,9 @@
 		if (utmp_chk(argv[1], argv[2]))
 			errx(1, "%s is not logged in on %s",
 			    argv[1], argv[2]);
-		ttyfd = term_chk(uid, argv[2], &msgsok, &atime, 0);
+		ttyfd = term_chk(uid, argv[2], &msgsok, &atime, 0, 1);
 		if (ttyfd == -1)
-			err(1, "%s%s", _PATH_DEV, argv[2]);
+			errx(1, "%s%s: Cannot access tty", _PATH_DEV, argv[2]);
 		if (myuid && !msgsok)
 			errx(1, "%s has messages disabled on %s",
 			    argv[1], argv[2]);
@@ -198,7 +198,7 @@
 	for (; ep; ep = ep->next)
 		if (strcmp(user, ep->name) == 0) {
 			++nloggedttys;
-			nfd = term_chk(uid, ep->line, &msgsok, &atime, 0);
+			nfd = term_chk(uid, ep->line, &msgsok, &atime, 0, 0);
 			if (nfd == -1)
 				continue;	/* bad term? skip */
 			if (myuid && !msgsok) {
@@ -240,18 +240,23 @@
  *     and the access time
  */
 int
-term_chk(uid_t uid, const char *tty, int *msgsokP, time_t *atimeP, int ismytty)
+term_chk(uid_t uid, const char *tty, int *msgsokP, time_t *atimeP, int ismytty,
+	int crashout)
 {
 	char path[MAXPATHLEN];
 	struct stat s;
 	int i, fd, serrno;
 
 	if (strcspn(tty, "./") != strlen(tty)) {
-		errno = EINVAL; return(-1);
+		if (crashout)
+			errx(1, "%s: Invalid tty name", tty);
+		return(-1);
 	}
 	i = snprintf(path, sizeof path, _PATH_DEV "%s", tty);
 	if (i < 0 || i >= sizeof(path)) {
-		errno = ENOMEM; return(-1);
+		if (crashout)
+			errx(1, "%s: Invalid tty name", tty);
+		return(-1);
 	}
 
 	(void)setegid(saved_egid);
@@ -260,12 +265,26 @@
 	(void)setegid(getgid());
 	errno = serrno;
 
-	if (fd == -1)
+	if (fd == -1) {
+		if (crashout)
+			err(1, "%s", path);
 		return(-1);
-	if (fstat(fd, &s) == -1)
+	}
+	if (fstat(fd, &s) == -1) {
+		if (crashout)
+			err(1, "%s: fstat", path);
 		goto error;
-	if (!isatty(fd) || s.st_uid != uid)
+	}
+	if (!isatty(fd)) {
+		if (crashout)
+			errx(1, "%s: Not a typewriter", path);
 		goto error;
+	}
+	if (s.st_uid != uid) {
+		if (crashout)
+			errx(1, "%s: Wrong ownership", path);
+		goto error;
+	}
 	*msgsokP = (s.st_mode & S_IWGRP) != 0;	/* group write bit */
 	*atimeP = s.st_atime;
 	if (ismytty)
@@ -273,9 +292,7 @@
 	return(ismytty? 0: fd);
 error:
 	if (fd != -1) {
-		serrno = errno;
 		close(fd);
-		errno = serrno;
 	}
 	return(-1);
 }
>Release-Note:
>Audit-Trail:
>Unformatted: