From: Todd C. Miller Date: Thu, 10 Apr 2014 22:11:47 +0000 (-0600) Subject: Make set_perms() and restore_perms() return an error instead of X-Git-Tag: SUDO_1_8_11^2~223 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a78da37487f3f6b2665b71d749629d800f92be4b;p=sudo Make set_perms() and restore_perms() return an error instead of calling exit() on failure. --- diff --git a/plugins/sudoers/ldap.c b/plugins/sudoers/ldap.c index 46aba7a0f..1d12c6a34 100644 --- a/plugins/sudoers/ldap.c +++ b/plugins/sudoers/ldap.c @@ -2107,9 +2107,11 @@ sudo_krb5_copy_cc_file(const char *old_ccname) old_ccname = sudo_krb5_ccname_path(old_ccname); if (old_ccname != NULL) { /* Open credential cache as user to prevent stolen creds. */ - set_perms(PERM_USER); + if (!set_perms(PERM_USER)) + goto done; ofd = open(old_ccname, O_RDONLY|O_NONBLOCK); - restore_perms(); + if (!restore_perms()) + goto done; if (ofd != -1) { (void) fcntl(ofd, F_SETFL, 0); @@ -2150,6 +2152,7 @@ write_error: "unable to open %s", old_ccname); } } +done: debug_return_str(ret); } diff --git a/plugins/sudoers/logging.c b/plugins/sudoers/logging.c index 5c82cb1b3..bc5acae25 100644 --- a/plugins/sudoers/logging.c +++ b/plugins/sudoers/logging.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994-1996, 1998-2013 Todd C. Miller + * Copyright (c) 1994-1996, 1998-2014 Todd C. Miller * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -236,6 +236,7 @@ log_denial(int status, bool inform_user) const char *message; char *logline; int oldlocale; + bool uid_changed; debug_decl(log_denial, SUDO_DEBUG_LOGGING) /* Handle auditing first (audit_failure() handles the locale itself). */ @@ -260,7 +261,7 @@ log_denial(int status, bool inform_user) debug_return; /* Become root if we are not already. */ - set_perms(PERM_ROOT|PERM_NOEXIT); + uid_changed = set_perms(PERM_ROOT); if (should_mail(status)) send_mail("%s", logline); /* send mail based on status */ @@ -273,7 +274,8 @@ log_denial(int status, bool inform_user) if (def_logfile) do_logfile(logline); - restore_perms(); + if (uid_changed) + restore_perms(); efree(logline); @@ -389,6 +391,7 @@ log_allowed(int status) { char *logline; int oldlocale; + bool uid_changed; debug_decl(log_allowed, SUDO_DEBUG_LOGGING) /* Log and mail messages should be in the sudoers locale. */ @@ -397,7 +400,7 @@ log_allowed(int status) logline = new_logline(NULL, 0); /* Become root if we are not already. */ - set_perms(PERM_ROOT|PERM_NOEXIT); + uid_changed = set_perms(PERM_ROOT); if (should_mail(status)) send_mail("%s", logline); /* send mail based on status */ @@ -410,7 +413,8 @@ log_allowed(int status) if (def_logfile) do_logfile(logline); - restore_perms(); + if (uid_changed) + restore_perms(); efree(logline); @@ -427,6 +431,7 @@ vlog_warning(int flags, const char *fmt, va_list ap) { int oldlocale, serrno = errno; char *logline, *message; + bool uid_changed; va_list ap2; debug_decl(vlog_error, SUDO_DEBUG_LOGGING) @@ -463,7 +468,7 @@ vlog_warning(int flags, const char *fmt, va_list ap) } /* Become root if we are not already. */ - set_perms(PERM_ROOT|PERM_NOEXIT); + uid_changed = set_perms(PERM_ROOT); /* * Send a copy of the error via mail. @@ -481,7 +486,8 @@ vlog_warning(int flags, const char *fmt, va_list ap) do_logfile(logline); } - restore_perms(); + if (uid_changed) + restore_perms(); efree(logline); @@ -688,10 +694,10 @@ send_mail(const char *fmt, ...) * (so user cannot kill it) or as the user (for the paranoid). */ #ifndef NO_ROOT_MAILER - set_perms(PERM_ROOT|PERM_NOEXIT); + (void) set_perms(PERM_ROOT); execve(mpath, argv, root_envp); #else - set_perms(PERM_FULL_USER|PERM_NOEXIT); + (void) set_perms(PERM_FULL_USER); execv(mpath, argv); #endif /* NO_ROOT_MAILER */ mysyslog(LOG_ERR, _("unable to execute %s: %m"), mpath); diff --git a/plugins/sudoers/parse.c b/plugins/sudoers/parse.c index 4fab89c6b..2508abecb 100644 --- a/plugins/sudoers/parse.c +++ b/plugins/sudoers/parse.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004-2005, 2007-2013 Todd C. Miller + * Copyright (c) 2004-2005, 2007-2014 Todd C. Miller * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -204,7 +204,8 @@ sudo_file_lookup(struct sudo_nss *nss, int validated, int pwflag) } /* Need to be runas user while stat'ing things. */ - set_perms(PERM_RUNAS); + if (!set_perms(PERM_RUNAS)) + debug_return_int(validated); match = UNSPEC; TAILQ_FOREACH_REVERSE(us, &userspecs, userspec_list, entries) { @@ -278,7 +279,7 @@ sudo_file_lookup(struct sudo_nss *nss, int validated, int pwflag) if (tags != NULL && tags->nopasswd != UNSPEC) def_authenticate = !tags->nopasswd; } - restore_perms(); + (void) restore_perms(); debug_return_int(validated); } diff --git a/plugins/sudoers/policy.c b/plugins/sudoers/policy.c index 46b676a80..257c5eea6 100644 --- a/plugins/sudoers/policy.c +++ b/plugins/sudoers/policy.c @@ -543,7 +543,7 @@ sudoers_policy_open(unsigned int version, sudo_conv_t conversation, if (fatal_setjmp() != 0) { /* called via fatal(), fatalx() or log_fatal() */ - rewind_perms(); + (void) rewind_perms(); fatal_disable_setjmp(); debug_return_bool(-1); } diff --git a/plugins/sudoers/set_perms.c b/plugins/sudoers/set_perms.c index a4b7af57e..a238a25c6 100644 --- a/plugins/sudoers/set_perms.c +++ b/plugins/sudoers/set_perms.c @@ -83,18 +83,20 @@ static int perm_stack_depth = 0; #undef OID #define OID(x) (ostate->x == state->x ? (uid_t)-1 : ostate->x) -void +bool rewind_perms(void) { debug_decl(rewind_perms, SUDO_DEBUG_PERMS) if (perm_stack_depth != 0) { - while (perm_stack_depth > 1) - restore_perms(); + while (perm_stack_depth > 1) { + if (!restore_perms()) + debug_return_bool(false); + } sudo_grlist_delref(perm_stack[0].grlist); } - debug_return; + debug_return_bool(true); } #if defined(HAVE_SETRESUID) @@ -108,18 +110,14 @@ rewind_perms(void) * We only flip the effective gid since it only changes for PERM_SUDOERS. * This version of set_perms() works fine with the "stay_setuid" option. */ -int +bool set_perms(int perm) { struct perm_state *state, *ostate = NULL; char errbuf[1024]; const char *errstr = errbuf; - int noexit; debug_decl(set_perms, SUDO_DEBUG_PERMS) - noexit = ISSET(perm, PERM_NOEXIT); - CLR(perm, PERM_MASK); - if (perm_stack_depth == PERM_STACK_MAX) { errstr = N_("perm stack overflow"); errno = EINVAL; @@ -361,25 +359,25 @@ set_perms(int perm) } perm_stack_depth++; - debug_return_bool(1); + debug_return_bool(true); bad: if (errno == EAGAIN) warningx(U_("%s: %s"), U_(errstr), U_("too many processes")); else warning("%s", U_(errstr)); - if (noexit) - debug_return_bool(0); - exit(1); + debug_return_bool(false); } -void +bool restore_perms(void) { struct perm_state *state, *ostate; debug_decl(restore_perms, SUDO_DEBUG_PERMS) - if (perm_stack_depth < 2) - debug_return; + if (perm_stack_depth < 2) { + warningx(U_("perm stack underflow")); + debug_return_bool(true); + } state = &perm_stack[perm_stack_depth - 1]; ostate = &perm_stack[perm_stack_depth - 2]; @@ -420,10 +418,10 @@ restore_perms(void) } } sudo_grlist_delref(state->grlist); - debug_return; + debug_return_bool(true); bad: - exit(1); + debug_return_bool(false); } #elif defined(_AIX) && defined(ID_SAVED) @@ -437,18 +435,14 @@ bad: * We only flip the effective gid since it only changes for PERM_SUDOERS. * This version of set_perms() works fine with the "stay_setuid" option. */ -int +bool set_perms(int perm) { struct perm_state *state, *ostate = NULL; char errbuf[1024]; const char *errstr = errbuf; - int noexit; debug_decl(set_perms, SUDO_DEBUG_PERMS) - noexit = ISSET(perm, PERM_NOEXIT); - CLR(perm, PERM_MASK); - if (perm_stack_depth == PERM_STACK_MAX) { errstr = N_("perm stack overflow"); errno = EINVAL; @@ -704,25 +698,25 @@ set_perms(int perm) } perm_stack_depth++; - debug_return_bool(1); + debug_return_bool(true); bad: if (errno == EAGAIN) warningx(U_("%s: %s"), U_(errstr), U_("too many processes")); else warning("%s", U_(errstr)); - if (noexit) - debug_return_bool(0); - exit(1); + debug_return_bool(false); } -void +bool restore_perms(void) { struct perm_state *state, *ostate; debug_decl(restore_perms, SUDO_DEBUG_PERMS) - if (perm_stack_depth < 2) - debug_return; + if (perm_stack_depth < 2) { + warningx(U_("perm stack underflow")); + debug_return_bool(true); + } state = &perm_stack[perm_stack_depth - 1]; ostate = &perm_stack[perm_stack_depth - 2]; @@ -827,10 +821,10 @@ restore_perms(void) } } sudo_grlist_delref(state->grlist); - debug_return; + debug_return_bool(true); bad: - exit(1); + debug_return_bool(false); } #elif defined(HAVE_SETREUID) @@ -844,18 +838,14 @@ bad: * We only flip the effective gid since it only changes for PERM_SUDOERS. * This version of set_perms() works fine with the "stay_setuid" option. */ -int +bool set_perms(int perm) { struct perm_state *state, *ostate = NULL; char errbuf[1024]; const char *errstr = errbuf; - int noexit; debug_decl(set_perms, SUDO_DEBUG_PERMS) - noexit = ISSET(perm, PERM_NOEXIT); - CLR(perm, PERM_MASK); - if (perm_stack_depth == PERM_STACK_MAX) { errstr = N_("perm stack overflow"); errno = EINVAL; @@ -1067,25 +1057,25 @@ set_perms(int perm) } perm_stack_depth++; - debug_return_bool(1); + debug_return_bool(true); bad: if (errno == EAGAIN) warningx(U_("%s: %s"), U_(errstr), U_("too many processes")); else warning("%s", U_(errstr)); - if (noexit) - debug_return_bool(0); - exit(1); + debug_return_bool(false); } -void +bool restore_perms(void) { struct perm_state *state, *ostate; debug_decl(restore_perms, SUDO_DEBUG_PERMS) - if (perm_stack_depth < 2) - debug_return; + if (perm_stack_depth < 2) { + warningx(U_("perm stack underflow")); + debug_return_bool(true); + } state = &perm_stack[perm_stack_depth - 1]; ostate = &perm_stack[perm_stack_depth - 2]; @@ -1129,10 +1119,10 @@ restore_perms(void) } } sudo_grlist_delref(state->grlist); - debug_return; + debug_return_bool(true); bad: - exit(1); + debug_return_bool(false); } #elif defined(HAVE_SETEUID) @@ -1145,18 +1135,14 @@ bad: * we are headed for an exec(). * This version of set_perms() works fine with the "stay_setuid" option. */ -int +bool set_perms(int perm) { struct perm_state *state, *ostate = NULL; char errbuf[1024]; const char *errstr = errbuf; - int noexit; debug_decl(set_perms, SUDO_DEBUG_PERMS) - noexit = ISSET(perm, PERM_NOEXIT); - CLR(perm, PERM_MASK); - if (perm_stack_depth == PERM_STACK_MAX) { errstr = N_("perm stack overflow"); errno = EINVAL; @@ -1367,25 +1353,25 @@ set_perms(int perm) } perm_stack_depth++; - debug_return_bool(1); + debug_return_bool(true); bad: if (errno == EAGAIN) warningx(U_("%s: %s"), U_(errstr), U_("too many processes")); else warning("%s", U_(errstr)); - if (noexit) - debug_return_bool(0); - exit(1); + debug_return_bool(false); } -void +bool restore_perms(void) { struct perm_state *state, *ostate; debug_decl(restore_perms, SUDO_DEBUG_PERMS) - if (perm_stack_depth < 2) - debug_return; + if (perm_stack_depth < 2) { + warningx(U_("perm stack underflow")); + debug_return_bool(true); + } state = &perm_stack[perm_stack_depth - 1]; ostate = &perm_stack[perm_stack_depth - 2]; @@ -1428,10 +1414,10 @@ restore_perms(void) goto bad; } sudo_grlist_delref(state->grlist); - debug_return; + debug_return_bool(true); bad: - exit(1); + debug_return_bool(false); } #else /* !HAVE_SETRESUID && !HAVE_SETREUID && !HAVE_SETEUID */ @@ -1441,18 +1427,14 @@ bad: * NOTE: does not support the "stay_setuid" or timestampowner options. * Also, sudoers_uid and sudoers_gid are not used. */ -int +bool set_perms(int perm) { struct perm_state *state, *ostate = NULL; char errbuf[1024]; const char *errstr = errbuf; - int noexit; debug_decl(set_perms, SUDO_DEBUG_PERMS) - noexit = ISSET(perm, PERM_NOEXIT); - CLR(perm, PERM_MASK); - if (perm_stack_depth == PERM_STACK_MAX) { errstr = N_("perm stack overflow"); errno = EINVAL; @@ -1535,25 +1517,25 @@ set_perms(int perm) } perm_stack_depth++; - debug_return_bool(1); + debug_return_bool(true); bad: if (errno == EAGAIN) warningx(U_("%s: %s"), U_(errstr), U_("too many processes")); else warning("%s", U_(errstr)); - if (noexit) - debug_return_bool(0); - exit(1); + debug_return_bool(false); } -void +boll restore_perms(void) { struct perm_state *state, *ostate; debug_decl(restore_perms, SUDO_DEBUG_PERMS) - if (perm_stack_depth < 2) - debug_return; + if (perm_stack_depth < 2) { + warningx(U_("perm stack underflow")); + debug_return_bool(true); + } state = &perm_stack[perm_stack_depth - 1]; ostate = &perm_stack[perm_stack_depth - 2]; @@ -1579,10 +1561,10 @@ restore_perms(void) warning("setuid(%d)", (int)ostate->ruid); goto bad; } - debug_return; + debug_return_bool(true); bad: - exit(1); + debug_return_bool(false); } #endif /* HAVE_SETRESUID || HAVE_SETREUID || HAVE_SETEUID */ diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 665cb30d4..7fbd01139 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -148,7 +148,8 @@ sudoers_policy_init(void *info, char * const envp[]) snl = sudo_read_nss(); /* LDAP or NSS may modify the euid so we need to be root for the open. */ - set_perms(PERM_ROOT); + if (!set_perms(PERM_ROOT)) + debug_return_bool(-1); /* Open and parse sudoers, set global defaults */ TAILQ_FOREACH_SAFE(nss, snl, entries, nss_next) { @@ -204,7 +205,8 @@ sudoers_policy_init(void *info, char * const envp[]) rval = true; cleanup: - restore_perms(); + if (!restore_perms()) + rval = -1; debug_return_bool(rval); } @@ -235,7 +237,8 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], goto bad; } - set_perms(PERM_INITIAL); + if (!set_perms(PERM_INITIAL)) + goto bad; /* Environment variables specified on the command line. */ if (env_add != NULL && env_add[0] != NULL) @@ -532,7 +535,8 @@ bad: done: fatal_disable_setjmp(); - rewind_perms(); + if (!rewind_perms()) + rval = -1; /* Close the password and group files and free up memory. */ sudo_endpwent(); @@ -600,7 +604,8 @@ init_vars(char * const envp[]) */ if (user_group_list == NULL) user_group_list = sudo_get_grlist(sudo_user.pw); - set_perms(PERM_INITIAL); + if (!set_perms(PERM_INITIAL)) + debug_return_bool(false); /* Set runas callback. */ sudo_defs_table[I_RUNAS_DEFAULT].callback = cb_runas_default; @@ -641,16 +646,20 @@ set_cmnd(void) if (ISSET(sudo_mode, MODE_RUN | MODE_CHECK)) { if (def_secure_path && !user_is_exempt()) path = def_secure_path; - set_perms(PERM_RUNAS); + if (!set_perms(PERM_RUNAS)) + debug_return_int(-1); rval = find_path(NewArgv[0], &user_cmnd, user_stat, path, def_ignore_dot); - restore_perms(); + if (!restore_perms()) + debug_return_int(-1); if (rval == NOT_FOUND) { /* Failed as root, try as invoking user. */ - set_perms(PERM_USER); + if (!set_perms(PERM_USER)) + debug_return_int(-1); rval = find_path(NewArgv[0], &user_cmnd, user_stat, path, def_ignore_dot); - restore_perms(); + if (!restore_perms()) + debug_return_int(-1); } if (rval == NOT_FOUND_ERROR) { if (errno == ENAMETOOLONG) @@ -721,7 +730,8 @@ open_sudoers(const char *sudoers, bool doedit, bool *keepopen) FILE *fp = NULL; debug_decl(open_sudoers, SUDO_DEBUG_PLUGIN) - set_perms(PERM_SUDOERS); + if (!set_perms(PERM_SUDOERS)) + debug_return_ptr(NULL); switch (sudo_secure_file(sudoers, sudoers_uid, sudoers_gid, &sb)) { case SUDO_PATH_SECURE: @@ -732,8 +742,8 @@ open_sudoers(const char *sudoers, bool doedit, bool *keepopen) */ if (sudoers_uid == ROOT_UID && ISSET(sudoers_mode, S_IRGRP)) { if (!ISSET(sb.st_mode, S_IRGRP) || sb.st_gid != SUDOERS_GID) { - restore_perms(); - set_perms(PERM_ROOT); + if (!restore_perms() || !set_perms(PERM_ROOT)) + debug_return_ptr(NULL); } } /* @@ -777,7 +787,11 @@ open_sudoers(const char *sudoers, bool doedit, bool *keepopen) break; } - restore_perms(); /* change back to root */ + if (!restore_perms()) { + /* unable to change back to root */ + fclose(fp); + fp = NULL; + } debug_return_ptr(fp); } @@ -1085,12 +1099,13 @@ create_admin_success_flag(void) debug_return; /* Create admin flag file if it doesn't already exist. */ - set_perms(PERM_USER); - if (stat(flagfile, &statbuf) != 0) { - fd = open(flagfile, O_CREAT|O_WRONLY|O_EXCL, 0644); - close(fd); + if (set_perms(PERM_USER)) { + if (stat(flagfile, &statbuf) != 0) { + fd = open(flagfile, O_CREAT|O_WRONLY|O_EXCL, 0644); + close(fd); + } + (void) restore_perms(); } - restore_perms(); debug_return; } #else /* !USE_ADMIN_FLAG */ diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index 4b8ff9dfa..35d7d7778 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -169,8 +169,6 @@ struct sudo_user { #define PERM_SUDOERS 0x04 #define PERM_RUNAS 0x05 #define PERM_TIMESTAMP 0x06 -#define PERM_NOEXIT 0x10 /* flag */ -#define PERM_MASK 0xf0 /* * Shortcuts for sudo_user contents. @@ -263,9 +261,9 @@ int sudo_file_display_bound_defaults(struct sudo_nss *, struct passwd *, struct int sudo_file_display_privs(struct sudo_nss *, struct passwd *, struct lbuf *); /* set_perms.c */ -void rewind_perms(void); -int set_perms(int); -void restore_perms(void); +bool rewind_perms(void); +bool set_perms(int); +bool restore_perms(void); int pam_prep_user(struct passwd *); /* gram.y */ diff --git a/plugins/sudoers/testsudoers.c b/plugins/sudoers/testsudoers.c index 60eba9530..9307f161f 100644 --- a/plugins/sudoers/testsudoers.c +++ b/plugins/sudoers/testsudoers.c @@ -463,15 +463,16 @@ init_envtables(void) return; } -int +bool set_perms(int perm) { - return 1; + return true; } -void +bool restore_perms(void) { + return true; } void diff --git a/plugins/sudoers/timestamp.c b/plugins/sudoers/timestamp.c index 265427481..dc40b622f 100644 --- a/plugins/sudoers/timestamp.c +++ b/plugins/sudoers/timestamp.c @@ -326,6 +326,7 @@ bool update_timestamp(struct passwd *pw) { struct timestamp_entry entry; + bool uid_changed = false; bool rval = false; int fd; debug_decl(update_timestamp, SUDO_DEBUG_AUTH) @@ -344,10 +345,10 @@ update_timestamp(struct passwd *pw) /* Open time stamp file and lock it for exclusive access. */ if (timestamp_uid != 0) - set_perms(PERM_TIMESTAMP); + uid_changed = set_perms(PERM_TIMESTAMP); fd = open(timestamp_file, O_RDWR|O_CREAT, 0600); - if (timestamp_uid != 0) - restore_perms(); + if (uid_changed) + (void) restore_perms(); if (fd == -1) { log_warning(USE_ERRNO, N_("unable to open %s"), timestamp_file); goto done; @@ -373,6 +374,7 @@ timestamp_status(struct passwd *pw) { struct timestamp_entry entry; struct timespec diff, timeout; + bool uid_changed = false; int status = TS_ERROR; /* assume the worst */ struct stat sb; int fd = -1; @@ -429,10 +431,10 @@ timestamp_status(struct passwd *pw) /* Open time stamp file and lock it for exclusive access. */ if (timestamp_uid != 0) - set_perms(PERM_TIMESTAMP); + uid_changed = set_perms(PERM_TIMESTAMP); fd = open(timestamp_file, O_RDWR); - if (timestamp_uid != 0) - restore_perms(); + if (uid_changed) + (void) restore_perms(); if (fd == -1) { status = TS_MISSING; goto done; @@ -524,6 +526,7 @@ void remove_timestamp(bool unlink_it) { struct timestamp_entry entry; + bool uid_changed = false; int fd = -1; debug_decl(remove_timestamp, SUDO_DEBUG_AUTH) @@ -559,10 +562,10 @@ remove_timestamp(bool unlink_it) /* Open time stamp file and lock it for exclusive access. */ if (timestamp_uid != 0) - set_perms(PERM_TIMESTAMP); + uid_changed = set_perms(PERM_TIMESTAMP); fd = open(timestamp_file, O_RDWR); - if (timestamp_uid != 0) - restore_perms(); + if (uid_changed) + (void) restore_perms(); if (fd == -1) goto done; lock_file(fd, SUDO_LOCK); @@ -616,6 +619,7 @@ bool set_lectured(void) { char lecture_status[PATH_MAX]; + bool uid_changed = false; int len, fd = -1; debug_decl(set_lectured, SUDO_DEBUG_AUTH) @@ -633,10 +637,10 @@ set_lectured(void) /* Create lecture file. */ if (timestamp_uid != 0) - set_perms(PERM_TIMESTAMP); + uid_changed = set_perms(PERM_TIMESTAMP); fd = open(lecture_status, O_WRONLY|O_CREAT|O_TRUNC, 0600); - if (timestamp_uid != 0) - restore_perms(); + if (uid_changed) + (void) restore_perms(); if (fd != -1) close(fd);