From 81f94499bf3711ddb83852e8dc3649dd8f8ce56c Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Thu, 25 Jun 2015 11:12:36 -0600 Subject: [PATCH] Check restore_perms() return value in all cases, pushing the return value back up the call stack. --- plugins/sudoers/check.c | 42 +++++++---- plugins/sudoers/check.h | 3 +- plugins/sudoers/ldap.c | 1 + plugins/sudoers/logging.c | 12 ++-- plugins/sudoers/parse.c | 3 +- plugins/sudoers/policy.c | 1 + plugins/sudoers/prompt.c | 4 +- plugins/sudoers/sudoers.c | 28 ++++---- plugins/sudoers/sudoers.h | 4 +- plugins/sudoers/timestamp.c | 140 +++++++++++++++++++++++------------- 10 files changed, 153 insertions(+), 85 deletions(-) diff --git a/plugins/sudoers/check.c b/plugins/sudoers/check.c index ff2c0842e..24f20316c 100644 --- a/plugins/sudoers/check.c +++ b/plugins/sudoers/check.c @@ -55,29 +55,37 @@ static struct passwd *get_authpw(int); static int check_user_interactive(int validated, int mode, struct passwd *auth_pw) { - int status, rval = true; + int status, rval = -1; + char *prompt; + bool lectured; debug_decl(check_user_interactive, SUDOERS_DEBUG_AUTH) /* Always need a password when -k was specified with the command. */ if (ISSET(mode, MODE_IGNORE_TICKET)) SET(validated, FLAG_CHECK_USER); - if (build_timestamp(auth_pw) == -1) { - rval = -1; + if (build_timestamp(auth_pw) == -1) goto done; - } status = timestamp_status(auth_pw); + switch (status) { + case TS_FATAL: + /* Fatal error (usually setuid failure), unsafe to proceed. */ + goto done; - if (status != TS_CURRENT || ISSET(validated, FLAG_CHECK_USER)) { - char *prompt; - bool lectured; + case TS_CURRENT: + /* Time stamp file is valid and current. */ + if (!ISSET(validated, FLAG_CHECK_USER)) { + rval = true; + break; + } + /* FALLTHROUGH */ + default: /* Bail out if we are non-interactive and a password is required */ if (ISSET(mode, MODE_NONINTERACTIVE)) { validated |= FLAG_NON_INTERACTIVE; log_auth_failure(validated, 0); - rval = -1; goto done; } @@ -87,20 +95,24 @@ check_user_interactive(int validated, int mode, struct passwd *auth_pw) /* Expand any escapes in the prompt. */ prompt = expand_prompt(user_prompt ? user_prompt : def_passprompt, auth_pw->pw_name); - if (prompt == NULL) { - rval = -1; + if (prompt == NULL) goto done; - } rval = verify_user(auth_pw, prompt, validated); - if (rval == true && lectured) - set_lectured(); + if (rval == true && lectured) { + if (set_lectured() == -1) + rval = -1; + } free(prompt); + break; } + /* Only update timestamp if user was validated. */ if (rval == true && ISSET(validated, VALIDATE_SUCCESS) && - !ISSET(mode, MODE_IGNORE_TICKET) && status != TS_ERROR) - update_timestamp(auth_pw); + !ISSET(mode, MODE_IGNORE_TICKET) && status != TS_ERROR) { + if (update_timestamp(auth_pw) == -1) + rval = -1; + } done: debug_return_int(rval); } diff --git a/plugins/sudoers/check.h b/plugins/sudoers/check.h index c8c48fc3c..5ef5b4f48 100644 --- a/plugins/sudoers/check.h +++ b/plugins/sudoers/check.h @@ -28,6 +28,7 @@ #define TS_MISSING 2 #define TS_NOFILE 3 #define TS_ERROR 4 +#define TS_FATAL 5 /* * Time stamps are now stored in a single file which contains multiple @@ -61,7 +62,7 @@ struct timestamp_entry { }; bool already_lectured(int status); -bool update_timestamp(struct passwd *pw); +int update_timestamp(struct passwd *pw); int build_timestamp(struct passwd *pw); int timestamp_status(struct passwd *pw); diff --git a/plugins/sudoers/ldap.c b/plugins/sudoers/ldap.c index 9222e5a73..0de60c3a6 100644 --- a/plugins/sudoers/ldap.c +++ b/plugins/sudoers/ldap.c @@ -2858,6 +2858,7 @@ sudo_ldap_bind_s(LDAP *ld) if (ldap_conf.krb5_ccname == NULL && user_ccname != NULL) { new_ccname = tmp_ccname = sudo_krb5_copy_cc_file(user_ccname); if (tmp_ccname == NULL) { + /* XXX - fatal error */ sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, "unable to copy user ccache %s", user_ccname); } diff --git a/plugins/sudoers/logging.c b/plugins/sudoers/logging.c index cd2322d51..8ac570a51 100644 --- a/plugins/sudoers/logging.c +++ b/plugins/sudoers/logging.c @@ -243,8 +243,10 @@ log_denial(int status, bool inform_user) if (def_logfile && !do_logfile(logline)) rval = false; - if (uid_changed) - restore_perms(); + if (uid_changed) { + if (!restore_perms()) + rval = false; /* XXX - return -1 instead? */ + } free(logline); @@ -385,8 +387,10 @@ log_allowed(int status) if (def_logfile && !do_logfile(logline)) rval = false; - if (uid_changed) - restore_perms(); + if (uid_changed) { + if (!restore_perms()) + rval = false; /* XXX - return -1 instead? */ + } free(logline); diff --git a/plugins/sudoers/parse.c b/plugins/sudoers/parse.c index 00f8f96f3..fe2804071 100644 --- a/plugins/sudoers/parse.c +++ b/plugins/sudoers/parse.c @@ -343,7 +343,8 @@ sudo_file_lookup(struct sudo_nss *nss, int validated, int pwflag) #if defined(HAVE_SELINUX) || defined(HAVE_PRIV_SET) done: #endif - (void) restore_perms(); + if (!restore_perms()) + SET(validated, VALIDATE_ERROR); debug_return_int(validated); } diff --git a/plugins/sudoers/policy.c b/plugins/sudoers/policy.c index 3db9c4093..5854ab7a6 100644 --- a/plugins/sudoers/policy.c +++ b/plugins/sudoers/policy.c @@ -722,6 +722,7 @@ sudoers_policy_invalidate(int remove) debug_decl(sudoers_policy_invalidate, SUDOERS_DEBUG_PLUGIN) user_cmnd = "kill"; + /* XXX - plugin API should support a return value for fatal errors. */ remove_timestamp(remove); sudoers_cleanup(); diff --git a/plugins/sudoers/prompt.c b/plugins/sudoers/prompt.c index eaec99057..608352e58 100644 --- a/plugins/sudoers/prompt.c +++ b/plugins/sudoers/prompt.c @@ -36,8 +36,8 @@ #include "sudoers.h" /* - * Expand %h and %u escapes in the prompt and pass back the dynamically - * allocated result. Returns the same string if there are no escapes. + * Expand %h and %u escapes (if present) in the prompt and pass back + * the dynamically allocated result. */ char * expand_prompt(const char *old_prompt, const char *auth_user) diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 256e43a93..ae910c208 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -74,7 +74,7 @@ static char *find_editor(int nfiles, char **files, int *argc_out, char ***argv_o static bool cb_runas_default(const char *); static bool cb_sudoers_locale(const char *); static int set_cmnd(void); -static void create_admin_success_flag(void); +static int create_admin_success_flag(void); static bool init_vars(char * const *); static bool set_fqdn(void); static bool set_loginclass(struct passwd *); @@ -465,7 +465,8 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], } /* Create Ubuntu-style dot file to indicate sudo was successful. */ - create_admin_success_flag(); + if (create_admin_success_flag() == -1) + goto bad; /* Finally tell the user if the command did not exist. */ if (cmnd_status == NOT_FOUND_DOT) { @@ -1209,39 +1210,42 @@ find_editor(int nfiles, char **files, int *argc_out, char ***argv_out) } #ifdef USE_ADMIN_FLAG -static void +static int create_admin_success_flag(void) { struct stat statbuf; char flagfile[PATH_MAX]; - int fd, n; + int len, fd = -1; debug_decl(create_admin_success_flag, SUDOERS_DEBUG_PLUGIN) /* Check whether the user is in the admin group. */ if (!user_in_group(sudo_user.pw, "admin")) - debug_return; + debug_return_int(true); /* Build path to flag file. */ - n = snprintf(flagfile, sizeof(flagfile), "%s/.sudo_as_admin_successful", + len = snprintf(flagfile, sizeof(flagfile), "%s/.sudo_as_admin_successful", user_dir); - if (n <= 0 || (size_t)n >= sizeof(flagfile)) - debug_return; + if (len <= 0 || (size_t)len >= sizeof(flagfile)) + debug_return_int(false); /* Create admin flag file if it doesn't already exist. */ if (set_perms(PERM_USER)) { if (stat(flagfile, &statbuf) != 0) { fd = open(flagfile, O_CREAT|O_WRONLY|O_EXCL, 0644); - close(fd); + if (fd != -1) + close(fd); } - (void) restore_perms(); + if (!restore_perms()) + debug_return_int(-1); } - debug_return; + debug_return_int(fd != -1); } #else /* !USE_ADMIN_FLAG */ -static void +static int create_admin_success_flag(void) { /* STUB */ + return true; } #endif /* USE_ADMIN_FLAG */ diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index 315fac425..af5e630fe 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -238,8 +238,8 @@ bool user_is_exempt(void); char *expand_prompt(const char *old_prompt, const char *auth_user); /* timestamp.c */ -void remove_timestamp(bool); -bool set_lectured(void); +int remove_timestamp(bool); +int set_lectured(void); /* sudo_auth.c */ bool sudo_auth_needs_end_session(void); diff --git a/plugins/sudoers/timestamp.c b/plugins/sudoers/timestamp.c index af5d000ba..3161a594d 100644 --- a/plugins/sudoers/timestamp.c +++ b/plugins/sudoers/timestamp.c @@ -44,6 +44,9 @@ #include "sudoers.h" #include "check.h" +#define TIMESTAMP_OPEN_ERROR -1 +#define TIMESTAMP_PERM_ERROR -2 + static char timestamp_file[PATH_MAX]; static off_t timestamp_hint = (off_t)-1; static struct timestamp_entry timestamp_key; @@ -306,17 +309,45 @@ build_timestamp(struct passwd *pw) debug_return_int(len); } +/* + * Open and lock the specified timestamp file. + * Returns 0 on success or -1 on failure. + */ +static int +open_timestamp(const char *path, int flags) +{ + bool uid_changed = false; + int fd; + debug_decl(open_timestamp, SUDOERS_DEBUG_AUTH) + + if (timestamp_uid != 0) + uid_changed = set_perms(PERM_TIMESTAMP); + fd = open(timestamp_file, flags, 0600); + if (uid_changed && !restore_perms()) { + /* Unable to restore permissions, should not happen. */ + if (fd != -1) { + int serrno = errno; + close(fd); + errno = serrno; + fd = TIMESTAMP_PERM_ERROR; + } + } + if (fd >= 0) + sudo_lock_file(fd, SUDO_LOCK); + + debug_return_int(fd); +} + /* * Update the time on the timestamp file/dir or create it if necessary. - * Returns true on success or false on failure. + * Returns true on success, false on failure or -1 on setuid failure. */ -bool +int update_timestamp(struct passwd *pw) { struct timestamp_entry entry; - bool uid_changed = false; - bool rval = false; - int fd; + int rval = false; + int fd = -1; debug_decl(update_timestamp, SUDOERS_DEBUG_AUTH) /* Zero timeout means don't update the time stamp file. */ @@ -335,25 +366,25 @@ update_timestamp(struct passwd *pw) } /* Open time stamp file and lock it for exclusive access. */ - if (timestamp_uid != 0) - uid_changed = set_perms(PERM_TIMESTAMP); - fd = open(timestamp_file, O_RDWR|O_CREAT, 0600); - if (uid_changed) - (void) restore_perms(); - if (fd == -1) { + fd = open_timestamp(timestamp_file, O_RDWR|O_CREAT); + switch (fd) { + case TIMESTAMP_OPEN_ERROR: log_warning(SLOG_SEND_MAIL, N_("unable to open %s"), timestamp_file); goto done; + case TIMESTAMP_PERM_ERROR: + /* Already logged set_perms/restore_perms error. */ + rval = -1; + goto done; } /* Update record or append a new one. */ - sudo_lock_file(fd, SUDO_LOCK); ts_update_record(fd, &entry, timestamp_hint); close(fd); rval = true; done: - debug_return_bool(rval); + debug_return_int(rval); } /* @@ -365,7 +396,6 @@ 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; @@ -424,16 +454,16 @@ timestamp_status(struct passwd *pw) goto done; /* Open time stamp file and lock it for exclusive access. */ - if (timestamp_uid != 0) - uid_changed = set_perms(PERM_TIMESTAMP); - fd = open(timestamp_file, O_RDWR); - if (uid_changed) - (void) restore_perms(); - if (fd == -1) { + fd = open_timestamp(timestamp_file, O_RDWR); + switch (fd) { + case TIMESTAMP_OPEN_ERROR: status = TS_MISSING; goto done; + case TIMESTAMP_PERM_ERROR: + /* Already logged set_perms/restore_perms error. */ + status = TS_FATAL; + goto done; } - sudo_lock_file(fd, SUDO_LOCK); /* Ignore and clear time stamp file if mtime predates boot time. */ if (fstat(fd, &sb) == 0) { @@ -516,22 +546,25 @@ done: /* * Remove the timestamp entry or file if unlink_it is set. + * Returns true on success, false on failure or -1 on setuid failure. + * A missing timestamp entry is not considered an error. */ -void +int remove_timestamp(bool unlink_it) { struct timestamp_entry entry; - bool uid_changed = false; - int fd = -1; + int fd, rval = true; debug_decl(remove_timestamp, SUDOERS_DEBUG_AUTH) - if (build_timestamp(NULL) == -1) - debug_return; + if (build_timestamp(NULL) == -1) { + rval = -1; + goto done; + } /* For "sudo -K" simply unlink the time stamp file. */ if (unlink_it) { - (void) unlink(timestamp_file); - debug_return; + rval = unlink(timestamp_file) ? -1 : true; + goto done; } /* @@ -556,14 +589,17 @@ remove_timestamp(bool unlink_it) } /* Open time stamp file and lock it for exclusive access. */ - if (timestamp_uid != 0) - uid_changed = set_perms(PERM_TIMESTAMP); - fd = open(timestamp_file, O_RDWR); - if (uid_changed) - (void) restore_perms(); - if (fd == -1) + fd = open_timestamp(timestamp_file, O_RDWR); + switch (fd) { + case TIMESTAMP_OPEN_ERROR: + if (errno != ENOENT) + rval = false; + goto done; + case TIMESTAMP_PERM_ERROR: + /* Already logged set_perms/restore_perms error. */ + rval = -1; goto done; - sudo_lock_file(fd, SUDO_LOCK); + } /* * Find matching entries and invalidate them. @@ -575,12 +611,13 @@ remove_timestamp(bool unlink_it) timestamp_hint -= (off_t)entry.size; /* Disable the entry. */ SET(entry.flags, TS_DISABLED); - ts_update_record(fd, &entry, timestamp_hint); + if (!ts_update_record(fd, &entry, timestamp_hint)) + rval = false; } close(fd); done: - debug_return; + debug_return_int(rval); } /* @@ -608,14 +645,13 @@ already_lectured(int unused) /* * Create the lecture status file. - * Returns true on success or false on failure. + * Returns true on success, false on failure or -1 on setuid failure. */ -bool +int set_lectured(void) { char lecture_status[PATH_MAX]; - bool uid_changed = false; - int len, fd = -1; + int len, fd, rval = false; debug_decl(set_lectured, SUDOERS_DEBUG_AUTH) len = snprintf(lecture_status, sizeof(lecture_status), "%s/%s", @@ -631,14 +667,22 @@ set_lectured(void) goto done; /* Create lecture file. */ - if (timestamp_uid != 0) - uid_changed = set_perms(PERM_TIMESTAMP); - fd = open(lecture_status, O_WRONLY|O_CREAT|O_TRUNC, 0600); - if (uid_changed) - (void) restore_perms(); - if (fd != -1) + fd = open_timestamp(lecture_status, O_RDWR|O_CREAT|O_TRUNC); + switch (fd) { + case TIMESTAMP_OPEN_ERROR: + /* Failed to open, not a fatal error. */ + break; + case TIMESTAMP_PERM_ERROR: + /* Already logged set_perms/restore_perms error. */ + rval = -1; + break; + default: + /* Success. */ close(fd); + rval = true; + break; + } done: - debug_return_bool(fd != -1 ? true : false); + debug_return_int(rval); } -- 2.49.0