[src/trunk]: src/usr.bin/calendar Correct privilege handling problems in cale...

[dholland <dholland%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/32edeec7735f
branches:  trunk
changeset: 339167:32edeec7735f
user:      dholland <dholland%NetBSD.org@localhost>
date:      Wed Jul 01 06:45:51 2015 +0000

description:
Correct privilege handling problems in calendar -a (which runs as root
from /etc/daily); do not exec other programs while the real uid is
still 0.

Also, clear the supplementary groups list up front and call initgroups
when becoming another user, to avoid leaking any extra group
privileges that we might have.

And finally, don't silently ignore errors changing uid and gid; those
are serious if they happen.

diffstat:

 usr.bin/calendar/calendar.c |  62 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 56 insertions(+), 6 deletions(-)

diffs (120 lines):

diff -r 3b2987c115a2 -r 32edeec7735f usr.bin/calendar/calendar.c
--- a/usr.bin/calendar/calendar.c       Wed Jul 01 03:39:36 2015 +0000
+++ b/usr.bin/calendar/calendar.c       Wed Jul 01 06:45:51 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: calendar.c,v 1.50 2013/11/09 15:57:15 christos Exp $   */
+/*     $NetBSD: calendar.c,v 1.51 2015/07/01 06:45:51 dholland Exp $   */
 
 /*
  * Copyright (c) 1989, 1993, 1994
@@ -39,7 +39,7 @@
 #if 0
 static char sccsid[] = "@(#)calendar.c 8.4 (Berkeley) 1/7/95";
 #endif
-__RCSID("$NetBSD: calendar.c,v 1.50 2013/11/09 15:57:15 christos Exp $");
+__RCSID("$NetBSD: calendar.c,v 1.51 2015/07/01 06:45:51 dholland Exp $");
 #endif /* not lint */
 
 #include <sys/param.h>
@@ -48,6 +48,7 @@
 #include <sys/uio.h>
 #include <sys/wait.h>
 
+#include <assert.h>
 #include <ctype.h>
 #include <err.h>
 #include <errno.h>
@@ -112,6 +113,7 @@
 static void     atodays(int, char *, unsigned short *);
 static void     cal(void);
 static void     closecal(FILE *);
+static void     changeuser(void);
 static int      getday(char *);
 static int      getfield(char *, char **, int *);
 static void     getmmdd(struct tm *, char *);
@@ -171,12 +173,24 @@
                 * XXX - This ignores the user's CALENDAR_DIR variable.
                 *       Run under user's login shell?
                 */
+               if (setgroups(0, NULL) == -1) {
+                       err(EXIT_FAILURE, "setgroups");
+               }
                while ((pw = getpwent()) != NULL) {
-                       (void)setegid(pw->pw_gid);
-                       (void)seteuid(pw->pw_uid);
-                       if (chdir(pw->pw_dir) != -1)
+                       if (setegid(pw->pw_gid) == -1) {
+                               warn("%s: setegid", pw->pw_name);
+                               continue;
+                       }
+                       if (seteuid(pw->pw_uid) == -1) {
+                               warn("%s: seteuid", pw->pw_name);
+                               continue;
+                       }
+                       if (chdir(pw->pw_dir) != -1) {
                                cal();
-                       (void)seteuid(0);
+                       }
+                       if (seteuid(0) == -1) {
+                               warn("%s: seteuid back to 0", pw->pw_name);
+                       }
                }
        } else if ((caldir = getenv("CALENDAR_DIR")) != NULL) {
                if (chdir(caldir) != -1)
@@ -429,6 +443,10 @@
                        (void)close(pdes[1]);
                }
                (void)close(pdes[0]);
+               if (doall) {
+                       /* become the user properly */
+                       changeuser();
+               }
                /* tell CPP to only open regular files */
                if(!cpp_restricted && setenv("CPP_RESTRICTED", "", 1) == -1)
                        err(EXIT_FAILURE, "Cannot restrict cpp");
@@ -495,6 +513,10 @@
                        (void)close(pdes[0]);
                }
                (void)close(pdes[1]);
+               if (doall) {
+                       /* become the user properly */
+                       changeuser();
+               }
                (void)execl(_PATH_SENDMAIL, "sendmail", "-i", "-t", "-F",
                    "\"Reminder Service\"", "-f", "root", NULL);
                err(EXIT_FAILURE, "Cannot exec `%s'", _PATH_SENDMAIL);
@@ -518,6 +540,34 @@
                continue;
 }
 
+static void
+changeuser(void)
+{
+       uid_t uid;
+       gid_t gid;
+
+       uid = geteuid();
+       gid = getegid();
+       assert(uid == pw->pw_uid);
+       assert(gid == pw->pw_gid);
+
+       if (seteuid(0) == -1) {
+               err(EXIT_FAILURE, "%s: changing user: cannot reassert uid 0",
+                   pw->pw_name);
+       }
+       if (setgid(gid) == -1) {
+               err(EXIT_FAILURE, "%s: cannot assume gid %d",
+                   pw->pw_name, (int)gid);
+       }
+       if (initgroups(pw->pw_name, gid) == -1) {
+               err(EXIT_FAILURE, "%s: cannot initgroups", pw->pw_name);
+       }
+       if (setuid(uid) == -1) {
+               err(EXIT_FAILURE, "%s: cannot assume uid %d",
+                   pw->pw_name, (int)uid);
+       }
+}
+
 static int
 getmonth(char *s)
 {