Security fix: correctly set groups according to the user. Previously,

root's groups' permissions were being used, so a user could read up to
16 (excluding initial whitespace) bytes of e.g. a wheel-accessible file.

Also, don't allow blocking on the opening of ~/.fakeid, so replace a fopen()
with open() and fdopen().  I knew I'd be going to hell for using C file
streams instead of POSIX syscalls...

1 files changed, 15 insertions, 6 deletions

diff --git a/usr.sbin/inetd/builtins.c b/usr.sbin/inetd/builtins.c
index 7d7d7a1..2a9b3b4 100644
--- a/usr.sbin/inetd/builtins.c
+++ b/usr.sbin/inetd/builtins.c
@@ -40,6 +40,7 @@
 #include <ctype.h>
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <pwd.h>
 #include <signal.h>
@@ -575,6 +576,7 @@ ident_stream(s, sep)		/* Ident service (AKA "auth") */
 	 */
 	if (fflag && !usedfallback) {
 		FILE *fakeid = NULL;
+		int fakeid_fd;
 
 		if (asprintf(&p, "%s/.fakeid", pw->pw_dir) == -1)
 			iderror(lport, fport, s, errno);
@@ -583,8 +585,9 @@ ident_stream(s, sep)		/* Ident service (AKA "auth") */
 		 * open any files we have no permission to open, especially
 		 * symbolic links to sensitive root-owned files or devices.
 		 */
+		if (initgroups(pw->pw_name, pw->pw_gid) == -1)
+			iderror(lport, fport, s, errno);
 		seteuid(pw->pw_uid);
-		setegid(pw->pw_gid);
 		/*
 		 * If we were to lstat() here, it would do no good, since it
 		 * would introduce a race condition and could be defeated.
@@ -592,9 +595,9 @@ ident_stream(s, sep)		/* Ident service (AKA "auth") */
 		 * and if it's not a regular file, we close it and end up
 		 * returning the user's real username.
 		 */
-		fakeid = fopen(p, "r");
+		fakeid_fd = open(p, O_RDONLY | O_NONBLOCK);
 		free(p);
-		if (fakeid != NULL &&
+		if ((fakeid = fdopen(fakeid_fd, "r")) != NULL &&
 		    fstat(fileno(fakeid), &sb) != -1 && S_ISREG(sb.st_mode)) {
 			buf[sizeof(buf) - 1] = '\0';
 			if (fgets(buf, sizeof(buf), fakeid) == NULL) {
@@ -605,7 +608,7 @@ ident_stream(s, sep)		/* Ident service (AKA "auth") */
 			fclose(fakeid);
 			/*
 			 * Usually, the file will have the desired identity
-			 * in the form "identity\n", so we use strtok() to
+			 * in the form "identity\n", so we use strcspn() to
 			 * end the string (which fgets() doesn't do.)
 			 */
 			buf[strcspn(buf, "\r\n")] = '\0';
@@ -624,10 +627,16 @@ ident_stream(s, sep)		/* Ident service (AKA "auth") */
 			 * we will return their real identity instead.
 			 */
 			
-			if (!*cp || getpwnam(cp))
-				cp = getpwuid(uc.cr_uid)->pw_name;
+			if (!*cp || getpwnam(cp)) {
+				pw = getpwuid(uc.cr_uid);
+				if (pw == NULL)
+					iderror(lport, fport, s, errno);
+				cp = pw->pw_name;
+			}
 		} else
 			cp = pw->pw_name;
+		if (fakeid_fd != -1)
+			close(fakeid_fd);
 	} else if (!usedfallback)
 		cp = pw->pw_name;
 	else