- Prevent users from hanging or killing 'calendar -a', at the cost of

one additional fork() per user who has a calendar file.  Side effect is
that root runs much less code.
- Use login caps so we don't, for example, give a user more processes than
he's allowed.
- tmpfile() rather than mkstemp() means never leaving junk behind.
- Don't let 'calendar -a' take more than a day.

Reviewed by millert@

3 files changed, 145 insertions, 39 deletions

diff --git a/usr.bin/calendar/calendar.c b/usr.bin/calendar/calendar.c
index abef570e653..553da968cb7 100644
--- a/usr.bin/calendar/calendar.c
+++ b/usr.bin/calendar/calendar.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: calendar.c,v 1.14 2000/11/21 14:01:38 aaron Exp $	*/
+/*	$OpenBSD: calendar.c,v 1.15 2001/09/03 15:53:00 pjanzen Exp $	*/
 
 /*
  * Copyright (c) 1989, 1993, 1994
@@ -43,14 +43,19 @@ static const char copyright[] =
 #if 0
 static const char sccsid[] = "@(#)calendar.c  8.3 (Berkeley) 3/25/94";
 #else
-static char rcsid[] = "$OpenBSD: calendar.c,v 1.14 2000/11/21 14:01:38 aaron Exp $";
+static char rcsid[] = "$OpenBSD: calendar.c,v 1.15 2001/09/03 15:53:00 pjanzen Exp $";
 #endif
 #endif /* not lint */
 
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include <err.h>
 #include <errno.h>
 #include <locale.h>
+#include <login_cap.h>
 #include <pwd.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -61,6 +66,10 @@ static char rcsid[] = "$OpenBSD: calendar.c,v 1.14 2000/11/21 14:01:38 aaron Exp
 #include "pathnames.h"
 #include "calendar.h"
 
+char *calendarFile = "calendar";  /* default calendar file */
+char *calendarHome = ".calendar"; /* HOME */
+char *calendarNoMail = "nomail";  /* don't sent mail if this file exists */
+
 struct passwd *pw;
 int doall = 0;
 time_t f_time = 0;
@@ -70,12 +79,14 @@ int f_dayBefore = 0; /* days before current date */
 
 struct specialev spev[NUMEV];
 
+void childsig __P((int));
+
 int
 main(argc, argv)
 	int argc;
 	char *argv[];
 {
-	int ch, i;
+	int ch;
 	char *caldir;
 
 	(void)setlocale(LC_ALL, "");
@@ -128,20 +139,110 @@ main(argc, argv)
 	settime(&f_time);
 
 	if (doall) {
+		pid_t kid, deadkid;
+		int kidstat, kidreaped, runningkids;
+		int acstat;
+		struct stat sbuf;
+		time_t t;
+		unsigned int sleeptime;
+
+		signal(SIGCHLD, childsig);
+		runningkids = 0;
+		t = time(NULL);
 		while ((pw = getpwent()) != NULL) {
-			(void)setlocale(LC_ALL, "");
-			(void)setegid(pw->pw_gid);
-			(void)initgroups(pw->pw_name, pw->pw_gid);
-			(void)seteuid(pw->pw_uid);
-			if (!chdir(pw->pw_dir)) {
+			acstat = 0;
+			/* Avoid unnecessary forks.  The calendar file is only
+			 * opened as the user later; if it can't be opened,
+			 * it's no big deal.  Also, get to correct directory.
+			 * Note that in an NFS environment root may get EACCES
+			 * on a chdir(), in which case we have to fork.  As long as
+			 * we can chdir() we can stat(), unless the user is
+			 * modifying permissions while this is running.
+			 */
+			if (chdir(pw->pw_dir)) {
+				if (errno == EACCES)
+					acstat = 1;
+				else
+					continue;
+			}
+			if (stat(calendarFile, &sbuf) != 0) {
+				if (chdir(calendarHome)) {
+					if (errno == EACCES)
+						acstat = 1;
+					else
+						continue;
+				}
+				if (stat(calendarNoMail, &sbuf) == 0 ||
+				    stat(calendarFile, &sbuf) != 0)
+					continue;
+			}
+			sleeptime = USERTIMEOUT;
+			switch ((kid = fork())) {
+			case -1:	/* error */
+				warn("fork");
+				continue;
+			case 0:	/* child */
+				(void)setlocale(LC_ALL, "");
+				if (setusercontext(NULL, pw, pw->pw_uid,
+				    LOGIN_SETALL ^ LOGIN_SETLOGIN))
+					err(1, "unable to set user context (uid %d)",
+					    (int)pw->pw_uid);
+				if (acstat) {
+					if (chdir(pw->pw_dir) ||
+					    stat(calendarFile, &sbuf) != 0 ||
+					    chdir(calendarHome) || 
+					    stat(calendarNoMail, &sbuf) == 0 ||
+					    stat(calendarFile, &sbuf) != 0)
+						exit(0);
+				}
 				cal();
-				/* Keep user settings from propogating */
-				for (i = 0; i < NUMEV; i++)
-					if (spev[i].uname != NULL)
-						free(spev[i].uname);
+				exit(0);
+			}
+			/* parent: wait a reasonable time, then kill child if
+			 * necessary.
+			 */
+			runningkids++;
+			kidreaped = 0;
+			do {
+				sleeptime = sleep(sleeptime);
+				/* Note that there is the possibility, if the sleep
+				 * stops early due to some other signal, of the child
+				 * terminating and not getting detected during the next
+				 * sleep.  In that unlikely worst case, we just sleep
+				 * too long for that user.
+				 */
+				for (;;) {
+					deadkid = waitpid(-1, &kidstat, WNOHANG);
+					if (deadkid <= 0)
+						break;
+					runningkids--;
+					if (deadkid == kid) {
+						kidreaped = 1;
+						sleeptime = 0;
+					}
+				}
+			} while (sleeptime);
+
+			if (!kidreaped) {
+				/* It doesn't _really_ matter if the kill fails, e.g.
+				 * if there's only a zombie now.
+				 */
+				(void)kill(kid, SIGTERM);
+				warnx("uid %d did not finish in time", (int)pw->pw_uid);
 			}
-			(void)seteuid(0);
+			if (time(NULL) - t >= SECSPERDAY)
+				errx(2, "'calendar -a' took more than a day; stopped at uid %d",
+				    (int)pw->pw_uid);
 		}
+		for (;;) {
+			deadkid = waitpid(-1, &kidstat, WNOHANG);
+			if (deadkid <= 0)
+				break;
+			runningkids--;
+		}
+		if (runningkids)
+			warnx(
+"%d child processes still running when 'calendar -a' finished", runningkids);
 	}
 	else if ((caldir = getenv("CALENDAR_DIR")) != NULL) {
 		if(!chdir(caldir))
@@ -161,3 +262,10 @@ usage()
 	    "[-f calendarfile]\n");
 	exit(1);
 }
+
+
+void
+childsig(sig)
+	int sig;
+{
+}
diff --git a/usr.bin/calendar/calendar.h b/usr.bin/calendar/calendar.h
index e16ae8677e0..2fe6d35225b 100644
--- a/usr.bin/calendar/calendar.h
+++ b/usr.bin/calendar/calendar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: calendar.h,v 1.4 1998/12/13 07:31:07 pjanzen Exp $	*/
+/*	$OpenBSD: calendar.h,v 1.5 2001/09/03 15:53:00 pjanzen Exp $	*/
 
 /*
  * Copyright (c) 1989, 1993, 1994
@@ -40,6 +40,7 @@ extern time_t f_time;
 extern struct iovec header[];
 extern struct tm *tp;
 extern char *calendarFile;
+extern char *calendarHome;
 extern char *optarg;
 
 struct fixs {
@@ -108,3 +109,9 @@ extern int f_dayBefore;	/* days before current date */
 
 #define NUMEV 2	/* Total number of such special events */
 extern struct specialev spev[NUMEV];
+
+/* For calendar -a, specify a maximum time (in seconds) to spend parsing
+ * each user's calendar files.  This prevents them from hanging calendar
+ * (e.g. by using named pipes)
+ */
+#define USERTIMEOUT 20
diff --git a/usr.bin/calendar/io.c b/usr.bin/calendar/io.c
index 2b6766a0507..6040f82096d 100644
--- a/usr.bin/calendar/io.c
+++ b/usr.bin/calendar/io.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: io.c,v 1.12 2001/07/09 07:04:48 deraadt Exp $	*/
+/*	$OpenBSD: io.c,v 1.13 2001/09/03 15:53:00 pjanzen Exp $	*/
 
 /*
  * Copyright (c) 1989, 1993, 1994
@@ -43,7 +43,7 @@ static const char copyright[] =
 #if 0
 static const char sccsid[] = "@(#)calendar.c  8.3 (Berkeley) 3/25/94";
 #else
-static char rcsid[] = "$OpenBSD: io.c,v 1.12 2001/07/09 07:04:48 deraadt Exp $";
+static char rcsid[] = "$OpenBSD: io.c,v 1.13 2001/09/03 15:53:00 pjanzen Exp $";
 #endif
 #endif /* not lint */
 
@@ -70,10 +70,6 @@ static char rcsid[] = "$OpenBSD: io.c,v 1.12 2001/07/09 07:04:48 deraadt Exp $";
 #include "calendar.h"
 
 
-char *calendarFile = "calendar";  /* default calendar file */
-char *calendarHome = ".calendar"; /* HOME */
-char *calendarNoMail = "nomail";  /* don't sent mail if this file exist */
-
 struct iovec header[] = {
 	{"From: ", 6},
 	{NULL, 0},
@@ -321,20 +317,12 @@ openf(path)
 FILE *
 opencal()
 {
-	int fd, pdes[2];
+	int pdes[2];
 	int fdin;
-	struct stat sbuf;
 
 	/* open up calendar file as stdin */
 	if ((fdin = openf(calendarFile)) == -1) {
-		if (doall) {
-			if (chdir(calendarHome) != 0)
-				return (NULL);
-			if (stat(calendarNoMail, &sbuf) == 0)
-				return (NULL);
-			if ((fdin = openf(calendarFile)) == -1)
-				return (NULL);
-		} else {
+		if (!doall) {
 			char *home = getenv("HOME");
 			if (home == NULL || *home == '\0')
 				errx(1, "cannot get home directory");
@@ -360,8 +348,17 @@ opencal()
 			(void)close(pdes[1]);
 		}
 		(void)close(pdes[0]);
-		(void)setuid(geteuid());
-		(void)setgid(getegid());
+		/* Set stderr to /dev/null.  Necessary so that cron does not
+		 * wait for cpp to finish if it's running calendar -a.
+		 */
+		if (doall) {
+			int fderr;
+			fderr = open(_PATH_DEVNULL, O_WRONLY, 0);
+			if (fderr == -1)
+				_exit(0);
+			(void)dup2(fderr, STDERR_FILENO);
+			(void)close(fderr);
+		}
 		execl(_PATH_CPP, "cpp", "-P", "-I.", _PATH_INCLUDE, (char *)NULL);
 		warn(_PATH_CPP);
 		_exit(1);
@@ -376,10 +373,7 @@ opencal()
 		return (stdout);
 
 	/* set output to a temporary file, so if no output don't send mail */
-	(void)snprintf(path, sizeof(path), "%s/_calXXXXXX", _PATH_TMP);
-	if ((fd = mkstemp(path)) < 0)
-		return (NULL);
-	return (fdopen(fd, "w+"));
+	return(tmpfile());
 }
 
 void
@@ -410,8 +404,6 @@ closecal(fp)
 			(void)close(pdes[0]);
 		}
 		(void)close(pdes[1]);
-		(void)setuid(geteuid());
-		(void)setgid(getegid());
 		execl(_PATH_SENDMAIL, "sendmail", "-i", "-t", "-F",
 		    "\"Reminder Service\"", (char *)NULL);
 		warn(_PATH_SENDMAIL);
@@ -427,7 +419,6 @@ closecal(fp)
 		(void)write(pdes[1], buf, nread);
 	(void)close(pdes[1]);
 done:	(void)fclose(fp);
-	(void)unlink(path);
 	while (wait(&status) >= 0)
 		;
 }