Check restore_perms() return value in all cases, pushing the

return value back up the call stack.

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) {
+    if (build_timestamp(auth_pw) == -1)
-	rval = -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)) {
+    case TS_CURRENT:
-	char *prompt;
+	/* Time stamp file is valid and current. */
-	bool lectured;
+	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) {
+	if (prompt == NULL)
-	    rval = -1;
 	    goto done;
-	}
 
 	rval = verify_user(auth_pw, prompt, validated);
-	if (rval == true && lectured)
+	if (rval == true && lectured) {
-	    set_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)
+	!ISSET(mode, MODE_IGNORE_TICKET) && status != TS_ERROR) {
-	update_timestamp(auth_pw);
+	if (update_timestamp(auth_pw) == -1)
+	    rval = -1;
+    }
 done:
     debug_return_int(rval);
 }

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);
 

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);
 	    }

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)
+    if (uid_changed) {
-	restore_perms();
+	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)
+    if (uid_changed) {
-	restore_perms();
+	if (!restore_perms())
+	    rval = false;		/* XXX - return -1 instead? */
+    }
 
     free(logline);
 

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);
 }
 

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();
 

plugins/sudoers/prompt.c

@@ -36,8 +36,8 @@
 #include "sudoers.h"
 
 /*
- * Expand %h and %u escapes in the prompt and pass back the dynamically
+ * Expand %h and %u escapes (if present) in the prompt and pass back
- * allocated result.  Returns the same string if there are no escapes.
+ * the dynamically allocated result.
  */
 char *
 expand_prompt(const char *old_prompt, const char *auth_user)

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))
+    if (len <= 0 || (size_t)len >= sizeof(flagfile))
-	debug_return;
+	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 */
 

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);
+int remove_timestamp(bool);
-bool set_lectured(void);
+int set_lectured(void);
 
 /* sudo_auth.c */
 bool sudo_auth_needs_end_session(void);

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;
@@ -307,16 +310,44 @@ build_timestamp(struct passwd *pw)
 }
 
 /*
- * Update the time on the timestamp file/dir or create it if necessary.
+ * Open and lock the specified timestamp file.
- * Returns true on success or false on failure.
+ * Returns 0 on success or -1 on failure.
  */
-bool
+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, false on failure or -1 on setuid failure.
+ */
+int
 update_timestamp(struct passwd *pw)
 {
     struct timestamp_entry entry;
-    bool uid_changed = false;
+    int rval = false;
-    bool rval = false;
+    int fd = -1;
-    int fd;
     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)
+    fd = open_timestamp(timestamp_file, O_RDWR|O_CREAT);
-	uid_changed = set_perms(PERM_TIMESTAMP);
+    switch (fd) {
-    fd = open(timestamp_file, O_RDWR|O_CREAT, 0600);
+    case TIMESTAMP_OPEN_ERROR:
-    if (uid_changed)
-	(void) restore_perms();
-    if (fd == -1) {
 	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)
+    fd = open_timestamp(timestamp_file, O_RDWR);
-	uid_changed = set_perms(PERM_TIMESTAMP);
+    switch (fd) {
-    fd = open(timestamp_file, O_RDWR);
+    case TIMESTAMP_OPEN_ERROR:
-    if (uid_changed)
-	(void) restore_perms();
-    if (fd == -1) {
 	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, rval = true;
-    int fd = -1;
     debug_decl(remove_timestamp, SUDOERS_DEBUG_AUTH)
 
-    if (build_timestamp(NULL) == -1)
+    if (build_timestamp(NULL) == -1) {
-	debug_return;
+	rval = -1;
+	goto done;
+    }
 
     /* For "sudo -K" simply unlink the time stamp file. */
     if (unlink_it) {
-	(void) unlink(timestamp_file);
+	rval = unlink(timestamp_file) ? -1 : true;
-	debug_return;
+	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)
+    fd = open_timestamp(timestamp_file, O_RDWR);
-	uid_changed = set_perms(PERM_TIMESTAMP);
+    switch (fd) {
-    fd = open(timestamp_file, O_RDWR);
+    case TIMESTAMP_OPEN_ERROR:
-    if (uid_changed)
+	if (errno != ENOENT)
-	(void) restore_perms();
+	    rval = false;
-    if (fd == -1)
 	goto done;
-    sudo_lock_file(fd, SUDO_LOCK);
+    case TIMESTAMP_PERM_ERROR:
+	/* Already logged set_perms/restore_perms error. */
+	rval = -1;
+	goto done;
+    }
 
     /*
      * 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, rval = false;
-    int len, fd = -1;
     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)
+    fd = open_timestamp(lecture_status, O_RDWR|O_CREAT|O_TRUNC);
-	uid_changed = set_perms(PERM_TIMESTAMP);
+    switch (fd) {
-    fd = open(lecture_status, O_WRONLY|O_CREAT|O_TRUNC, 0600);
+    case TIMESTAMP_OPEN_ERROR:
-    if (uid_changed)
+	/* Failed to open, not a fatal error. */
-	(void) restore_perms();
+	break;
-    if (fd != -1)
+    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);
 }