Run most of the code as root, not the invoking user. It doesn't really

gain us anything to run as the user since an attacker can just have
an setuid(0) in their egg.  Running as root solves potential problems
wrt signalling.

diff --git a/auth/API b/auth/API
index 88991b66c6bbbd259ba83e9d7475565270400486..fcb2f1b6d3d91041d5064792d018d73b9246d4e0 100644 (file)
--- a/auth/API
+++ b/auth/API
@@ -31,8 +31,8 @@ The variables in the struct are as follows:
                initialized in the "init" or "setup" routines.
 
 Possible values of sudo_auth.flags:
-    FLAG_ROOT          Whether or not the auth functions should run with
-                       an euid of 0 or the uid of the invoking user.
+    FLAG_USER          Whether or not the auth functions should run with
+                       the euid of the invoking user instead of 0.
 
     FLAG_CONFIGURED    If set then the auth method is assumed to have been
                        configured successfully.  All auth methods start out
@@ -102,23 +102,23 @@ in sudo_auth.h.  For instance, for a method, ``fooauth'', add:
 
 #elif defined(HAVE_FOOAUTH)
 #  define AUTH_STANDALONE \
-        AUTH_ENTRY(FLAG_ROOT, "foo", \
+        AUTH_ENTRY(0, "foo", \
            foo_init, foo_setup, foo_verify, foo_cleanup)
 
-If the method doesn't need to run as root, replace FLAG_ROOT with 0.
-If you don't have a init/setup/cleanup routine, just use a NULL for that
-field.
+If the method needs to run as the user, not root, replace the first
+parameter to AUTH_ENTRY (0) with FLAG_USER.  If you don't have a
+init/setup/cleanup routine, just use a NULL for that field.
 
 For a normal authentication method, add it to the ``auth_switch'' in
 sudo_auth.c.  If ``fooauth'' is a normal auth method, its entry
 would look like:
 
 #  ifdef HAVE_FOOAUTH
-    AUTH_ENTRY(FLAG_ROOT, "foo", foo_init, foo_setup, foo_verify, foo_cleanup)
+    AUTH_ENTRY(0, "foo", foo_init, foo_setup, foo_verify, foo_cleanup)
 #  endif
 
-Again, if the method doesn't need to run as root, replace FLAG_ROOT
-with 0.  Likewise, if you don't have a init/setup/cleanup routine,
+Again, if the method doesn't need to run as root, replace the 0 with
+FLAG_USER.  Likewise, if you don't have a init/setup/cleanup routine,
 just use a NULL for that field.
 
 NOTE:  You should not make a method both ``standalone'' and

diff --git a/auth/sudo_auth.c b/auth/sudo_auth.c
index b93ef19a340307fecd1e3e8971afeab5dd760ab0..ff14ec7c8968622474014f9ee70c46d0a53d3124 100644 (file)
--- a/auth/sudo_auth.c
+++ b/auth/sudo_auth.c
@@ -65,25 +65,25 @@ sudo_auth auth_switch[] = {
     AUTH_STANDALONE
 #else
 #  ifndef WITHOUT_PASSWD
-    AUTH_ENTRY(FLAG_ROOT, "passwd", NULL, NULL, passwd_verify, NULL)
+    AUTH_ENTRY(0, "passwd", NULL, NULL, passwd_verify, NULL)
 #  endif
 #  if defined(HAVE_SECUREWARE) && !defined(WITHOUT_PASSWD)
-    AUTH_ENTRY(FLAG_ROOT, "secureware", secureware_init, NULL, secureware_verify, NULL)
+    AUTH_ENTRY(0, "secureware", secureware_init, NULL, secureware_verify, NULL)
 #  endif
 #  ifdef HAVE_AFS
-    AUTH_ENTRY(FLAG_ROOT, "afs", NULL, NULL, afs_verify, NULL)
+    AUTH_ENTRY(0, "afs", NULL, NULL, afs_verify, NULL)
 #  endif
 #  ifdef HAVE_KERB4
-    AUTH_ENTRY(FLAG_ROOT, "kerb4", kerb4_init, NULL, kerb4_verify, NULL)
+    AUTH_ENTRY(0, "kerb4", kerb4_init, NULL, kerb4_verify, NULL)
 #  endif
 #  ifdef HAVE_KERB5
-    AUTH_ENTRY(FLAG_ROOT, "kerb5", kerb5_init, NULL, kerb5_verify, NULL)
+    AUTH_ENTRY(0, "kerb5", kerb5_init, NULL, kerb5_verify, NULL)
 #  endif
 #  ifdef HAVE_SKEY
-    AUTH_ENTRY(FLAG_ROOT, "S/Key", NULL, rfc1938_setup, rfc1938_verify, NULL)
+    AUTH_ENTRY(0, "S/Key", NULL, rfc1938_setup, rfc1938_verify, NULL)
 #  endif
 #  ifdef HAVE_OPIE
-    AUTH_ENTRY(FLAG_ROOT, "OPIE", NULL, rfc1938_setup, rfc1938_verify, NULL)
+    AUTH_ENTRY(0, "OPIE", NULL, rfc1938_setup, rfc1938_verify, NULL)
 #  endif
 #endif /* AUTH_STANDALONE */
     AUTH_ENTRY(0, NULL, NULL, NULL, NULL, NULL)
@@ -107,8 +107,8 @@ verify_user()
     /* Initialize auth methods and unconfigure the method if necessary. */
     for (auth = auth_switch; auth->name; auth++) {
        if (auth->init && IS_CONFIGURED(auth)) {
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_ROOT, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_USER, 0);
 
            status = (auth->init)(sudo_user.pw, &user_prompt, auth);
            if (status == AUTH_FAILURE)
@@ -116,8 +116,8 @@ verify_user()
            else if (status == AUTH_FATAL)      /* XXX log */
                exit(1);                /* assume error msg already printed */
 
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_USER, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_ROOT, 0);
        }
     }
 
@@ -125,8 +125,8 @@ verify_user()
        /* Do any per-method setup and unconfigure the method if needed */
        for (auth = auth_switch; auth->name; auth++) {
            if (auth->setup && IS_CONFIGURED(auth)) {
-               if (NEEDS_ROOT(auth))
-                   set_perms(PERM_ROOT, 0);
+               if (NEEDS_USER(auth))
+                   set_perms(PERM_USER, 0);
 
                status = (auth->setup)(sudo_user.pw, &user_prompt, auth);
                if (status == AUTH_FAILURE)
@@ -134,8 +134,8 @@ verify_user()
                else if (status == AUTH_FATAL)  /* XXX log */
                    exit(1);            /* assume error msg already printed */
 
-               if (NEEDS_ROOT(auth))
-                   set_perms(PERM_USER, 0);
+               if (NEEDS_USER(auth))
+                   set_perms(PERM_ROOT, 0);
            }
        }
 
@@ -154,14 +154,14 @@ verify_user()
            if (!IS_CONFIGURED(auth))
                continue;
 
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_ROOT, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_USER, 0);
 
            success = auth->status = (auth->verify)(sudo_user.pw, p, auth);
            (void) memset(p, 0, strlen(p));
 
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_USER, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_ROOT, 0);
 
            if (auth->status != AUTH_FAILURE)
                goto cleanup;
@@ -182,15 +182,15 @@ cleanup:
     /* Call cleanup routines. */
     for (auth = auth_switch; auth->name; auth++) {
        if (auth->cleanup && IS_CONFIGURED(auth)) {
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_ROOT, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_USER, 0);
 
            status = (auth->cleanup)(sudo_user.pw, auth);
            if (status == AUTH_FATAL)   /* XXX log */
                exit(1);                /* assume error msg already printed */
 
-           if (NEEDS_ROOT(auth))
-               set_perms(PERM_USER, 0);
+           if (NEEDS_USER(auth))
+               set_perms(PERM_ROOT, 0);
        }
     }
 

diff --git a/auth/sudo_auth.h b/auth/sudo_auth.h
index 821822cf464ea5ef5fed752058fff95c2d892975..e6aa3ef4e470fbac294ebcdd74a92546d3fb7dbb 100644 (file)
--- a/auth/sudo_auth.h
+++ b/auth/sudo_auth.h
@@ -55,12 +55,12 @@ typedef struct sudo_auth {
 
 /* Values for sudo_auth.flags.  */
 /* XXX - these names are too long for my liking */
-#define FLAG_ROOT      0x01    /* functions must run as root */
+#define FLAG_USER      0x01    /* functions must run as root */
 #define FLAG_CONFIGURED        0x02    /* method configured ok */
 #define FLAG_ONEANDONLY        0x04    /* one and only auth method */
 
 /* Shortcuts for using the flags above. */
-#define NEEDS_ROOT(x)          ((x)->flags & FLAG_ROOT)
+#define NEEDS_USER(x)          ((x)->flags & FLAG_USER)
 #define IS_CONFIGURED(x)       ((x)->flags & FLAG_CONFIGURED)
 #define IS_ONEANDONLY(x)       ((x)->flags & FLAG_ONEANDONLY)
 
@@ -99,27 +99,27 @@ int securid_verify __P((struct passwd *pw, char *pass, sudo_auth *auth));
 /* Some methods cannots (or should not) interoperate with any others */
 #if defined(HAVE_PAM)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "pam", \
+       AUTH_ENTRY(0, "pam", \
            pam_init, NULL, pam_verify, pam_cleanup)
 #elif defined(HAVE_SECURID)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "SecurId", \
+       AUTH_ENTRY(0, "SecurId", 
            securid_init, securid_setup, securid_verify, NULL)
 #elif defined(HAVE_SIA)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "sia", \
+       AUTH_ENTRY(0, "sia", \
            NULL, sia_setup, sia_verify, sia_cleanup)
 #elif defined(HAVE_DCE)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "dce", \
+       AUTH_ENTRY(0, "dce", \
            NULL, NULL, dce_verify, NULL)
 #elif defined(HAVE_AUTHENTICATE)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "aixauth", \
+       AUTH_ENTRY(0, "aixauth", \
            NULL, NULL, aixauth_verify, NULL)
 #elif defined(HAVE_FWTK)
 #  define AUTH_STANDALONE \
-       AUTH_ENTRY(FLAG_ROOT, "fwtk", fwtk_init, \
+       AUTH_ENTRY(0, "fwtk", fwtk_init, \
            NULL, fwtk_verify, fwtk_cleanup)
 #endif
 

diff --git a/check.c b/check.c
index 8fb32e1a2788cde539fbc4afecf6fd59dcf003f1..170f029916063ccf35622bb98114585eac4f4859 100644 (file)
--- a/check.c
+++ b/check.c
@@ -137,8 +137,6 @@ update_timestamp(timestampdir, timestampfile)
     char *timestampfile;
 {
 
-    set_perms(PERM_ROOT, 0);           /* become root */
-
     if (touch(timestampfile ? timestampfile : timestampdir, time(NULL)) == -1) {
        if (timestampfile) {
            int fd = open(timestampfile, O_WRONLY|O_CREAT|O_TRUNC, 0600);
@@ -152,8 +150,6 @@ update_timestamp(timestampdir, timestampfile)
                log_error(NO_EXIT|USE_ERRNO, "Can't mkdir %s", timestampdir);
        }
     }
-
-    set_perms(PERM_USER, 0);           /* relinquish root */
 }
 
 /*
@@ -288,8 +284,6 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs)
     time_t now;
     int status = TS_ERROR;             /* assume the worst */
 
-    set_perms(PERM_ROOT, 0);           /* become root */
-
     /*
      * Sanity check _PATH_SUDO_TIMEDIR and make it if it doesn't already exist.
      * We start out assuming the worst (that the dir is not sane) and
@@ -325,10 +319,8 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs)
                status = TS_MISSING;
        }
     }
-    if (status == TS_ERROR) {
-       set_perms(PERM_USER, 0);                /* relinquish root */
+    if (status == TS_ERROR)
        return(status);
-    }
 
     /*
      * Sanity check the user's ticket dir.  We start by downgrading
@@ -435,7 +427,6 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs)
        }
     }
 
-    set_perms(PERM_USER, 0);           /* relinquish root */
     return(status);
 }
 
@@ -455,7 +446,6 @@ remove_timestamp(remove)
     status = timestamp_status(timestampdir, timestampfile, user_name, FALSE);
     if (status == TS_OLD || status == TS_CURRENT) {
        ts = timestampfile ? timestampfile : timestampdir;
-       set_perms(PERM_ROOT, 0);                /* become root */
        if (remove) {
            if (timestampfile)
                status = unlink(timestampfile);
@@ -471,7 +461,6 @@ remove_timestamp(remove)
            (void) fprintf(stderr, "%s: can't reset %s to epoch: %s\n",
                Argv[0], ts, strerror(errno));
        }
-       set_perms(PERM_USER, 0);                /* relinquish root */
     }
 
     (void) free(timestampdir);

diff --git a/find_path.c b/find_path.c
index cf6342907efc0d1236fe67eca7b7563d2d77c010..be92d3bba18170d2c791dd682905749dcca3cefb 100644 (file)
--- a/find_path.c
+++ b/find_path.c
@@ -112,7 +112,6 @@ find_path(infile, outfile)
     path = estrdup(path);
     origpath = path;
 
-    /* XXX use strtok() */
     do {
        if ((n = strchr(path, ':')))
            *n = '\0';

diff --git a/goodpath.c b/goodpath.c
index 4808da9df85785d9b80708ac0198918c6e5b22d2..f950d944201a372a4c33a19967fe2a4efb2f74c8 100644 (file)
--- a/goodpath.c
+++ b/goodpath.c
@@ -67,19 +67,12 @@ sudo_goodpath(path)
     const char *path;
 {
     struct stat sb;
-    int err;
 
     /* Check for brain damage */
     if (path == NULL || path[0] == '\0')
        return(NULL);
 
-    /* Do the stat() as root. */
-    set_perms(PERM_ROOT, 0);
-    err = stat(path, &sb);
-    set_perms(PERM_USER, 0);
-
-    /* stat() failed */
-    if (err)
+    if (stat(path, &sb))
        return(NULL);
 
     /* Make sure path describes an executable regular file. */

diff --git a/logging.c b/logging.c
index 74174e11d3294119b89de8da2b93edad2d395c4b..bf8743c28b1bd11cf75d958f3249c18bf2fe5928 100644 (file)
--- a/logging.c
+++ b/logging.c
@@ -161,15 +161,10 @@ do_logfile(msg)
     FILE *fp;
     mode_t oldmask;
     time_t now;
-    int oldeuid = geteuid();
     int maxlen = MAXLOGFILELEN;
 
     now = time((time_t) 0);
 
-    /* Become root if we are not already. */
-    if (oldeuid)
-       set_perms(PERM_ROOT, 0);
-
     oldmask = umask(077);
     fp = fopen(_PATH_SUDO_LOGFILE, "a");
     (void) umask(oldmask);
@@ -254,9 +249,6 @@ do_logfile(msg)
        (void) lock_file(fileno(fp), SUDO_UNLOCK);
        (void) fclose(fp);
     }
-
-    if (oldeuid)
-       set_perms(PERM_USER, 0);        /* relinquish root */
 }
 #endif /* LOGGING & SLOG_FILE */
 
@@ -299,20 +291,18 @@ log_auth(status, inform_user)
     mail_auth(status, logline);                /* send mail based on status */
 
     /* Inform the user if they failed to authenticate.  */
-    if (inform_user) {
+    if (inform_user && (status & VALIDATE_NOT_OK)) {
        if (status & FLAG_NO_USER)
            (void) fprintf(stderr, "%s is not in the sudoers file.  %s",
                user_name, "This incident will be reported.\n");
        else if (status & FLAG_NO_HOST)
            (void) fprintf(stderr, "%s is not allowed to run sudo on %s.  %s",
                user_name, user_shost, "This incident will be reported.\n");
-       else if (status & VALIDATE_NOT_OK)
+       else
            (void) fprintf(stderr,
                "Sorry, user %s is not allowed to execute '%s%s%s' as %s on %s.\n",
                user_name, user_cmnd, user_args ? " " : "",
                user_args ? user_args : "", user_runas, user_host);
-       else
-           (void) fprintf(stderr, "An unknown error has occurred.\n");
     }
 
     /*
@@ -351,6 +341,10 @@ log_error(va_alist)
     fmt = va_arg(ap, const char *);
 #endif
 
+    /* Become root if we are not already to avoid user control */
+    if (geteuid() != 0)
+       set_perms(PERM_ROOT, 0);
+
     /* Expand printf-style format + args. */
     evasprintf(&message, fmt, ap);
     va_end(ap);
@@ -409,6 +403,9 @@ log_error(va_alist)
        free(message);
 }
 
+/*
+ * Send a message to ALERTMAIL
+ */
 #ifdef _PATH_SENDMAIL
 static void
 send_mail(line)
@@ -481,7 +478,7 @@ send_mail(line)
            user_name, line);
        fclose(mail);
        reapchild(0);
-       exit(0);
+       _exit(0);
     } else {
        /* Parent, just return unless there is an error. */
        if (pid == -1) {
@@ -510,6 +507,7 @@ mail_auth(status, line)
 {
     int mail_mask;
 
+    /* If any of these bits are set in status, we send mail. */
     mail_mask = VALIDATE_ERROR;
 #ifdef SEND_MAIL_WHEN_OK
     mail_mask |= VALIDATE_OK;

diff --git a/parse.c b/parse.c
index 51daa8f5199f5cb47206e31c40ed3fc200329d41..62bc842f8ccdc3db952e124c426f9289fb4affab 100644 (file)
--- a/parse.c
+++ b/parse.c
@@ -124,26 +124,17 @@ sudoers_lookup(check_cmnd)
     yyin = sudoers_fp;
     yyout = stdout;
 
-    /*
-     * Allocate space for data structures in the parser.
-     */
+    /* Allocate space for data structures in the parser. */
     init_parser();
 
-    /*
-     * Need to be root while stat'ing things in the parser.
-     */
+    /* Need to be root while stat'ing things in the parser. */
     set_perms(PERM_ROOT, 0);
     error = yyparse();
 
-    /*
-     * Don't need to keep this open...
-     */
+    /* Close the sudoers file now that we are done with it. */
     (void) fclose(sudoers_fp);
     sudoers_fp = NULL;
 
-    /* relinquish extra privs */
-    set_perms(PERM_USER, 0);
-
     if (error || parse_error)
        return(VALIDATE_ERROR);
 
@@ -159,10 +150,9 @@ sudoers_lookup(check_cmnd)
     }
 
     /*
-     * Only check the actual command if the check_cmnd
-     * flag is set.  It is not set for the "validate"
-     * and "list" pseudo-commands.  Always check the
-     * host and user.
+     * Only check the actual command if the check_cmnd flag is set.
+     * It is not set for the "validate" and "list" pseudo-commands.
+     * Always check the host and user.
      */
     if (check_cmnd == FALSE)
        while (top) {

diff --git a/sudo.c b/sudo.c
index 06b9c34ea1cdb7242b335a697f87fc2f292667bc..c8b68b15247e9afef1b729dee0a894f0344c87bb 100644 (file)
--- a/sudo.c
+++ b/sudo.c
@@ -258,7 +258,7 @@ main(argc, argv)
 
     cmnd_status = init_vars(sudo_mode);
 
-    set_perms(PERM_USER, sudo_mode);
+    /* At this point, ruid == euid == 0 */
 
     check_sudoers();   /* check mode/owner on _PATH_SUDOERS */
 
@@ -763,7 +763,6 @@ check_sudoers()
      * Fix the mode and group on sudoers file from old default.
      * Only works if filesystem is readable/writable by root.
      */
-    set_perms(PERM_ROOT, 0);
     if ((rootstat = lstat(_PATH_SUDOERS, &statbuf)) == 0 &&
        SUDOERS_UID == statbuf.st_uid && SUDOERS_MODE != 0400 &&
        (statbuf.st_mode & 0007777) == 0400) {
@@ -824,8 +823,7 @@ check_sudoers()
            log_error(USE_ERRNO, "can't open %s", _PATH_SUDOERS);
     }
 
-    set_perms(PERM_ROOT, 0);
-    set_perms(PERM_USER, 0);
+    set_perms(PERM_ROOT, 0);           /* change back to root */
 }
 
 /*