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

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

details:   https://anonhg.NetBSD.org/src/rev/25bbc9ee99b3
branches:  trunk
changeset: 809309:25bbc9ee99b3
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 465b43340df3 -r 25bbc9ee99b3 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)
 {