# HG changeset patch # User markus schnalke # Date 1314454747 -7200 # Node ID e507c854a63ea9e67c2297473d97ccc418ab2e77 # Parent f899ca0835a81edcf3264f97f54a01f3b0172e70 Security fix! Correct handling of seteuid() return value See Debian bug #638002, reported by John Lightsey. When possible the (already available) set_euidgid() function is used. Additionally, it is unnecessary to change the identity when writing into an already open file descriptor. This should fix the problem. diff -r f899ca0835a8 -r e507c854a63e src/log.c --- a/src/log.c Wed Jul 13 10:30:52 2011 +0200 +++ b/src/log.c Sat Aug 27 16:19:07 2011 +0200 @@ -65,8 +65,9 @@ uid_t saved_uid; gid_t saved_gid; - saved_gid = setegid(conf.mail_gid); - saved_uid = seteuid(conf.mail_uid); + if (!conf.run_as_user) { + set_euidgid(conf.mail_uid, conf.mail_gid, &saved_uid, &saved_gid); + } filename = g_strdup_printf("%s/masqmail.log", conf.log_dir); logfile = fopen(filename, "a"); @@ -76,8 +77,9 @@ } g_free(filename); - seteuid(saved_uid); - setegid(saved_gid); + if (!conf.run_as_user) { + set_euidgid(saved_uid, saved_gid, NULL, NULL); + } } #ifdef ENABLE_DEBUG @@ -114,35 +116,26 @@ va_copy(args_copy, args); vfprintf(stdout, fmt, args_copy); va_end(args_copy); - fflush(stdout); /* is this necessary? */ + fflush(stdout); /* in case output ends not with newline */ } pri &= ~LOG_VERBOSE; - if (pri) { - if (conf.use_syslog) - vsyslog(pri, fmt, args); - else { - if (pri <= conf.log_max_pri) { - FILE *file = logfile ? logfile : stderr; - time_t now = time(NULL); - struct tm *t = localtime(&now); - gchar buf[24]; - uid_t saved_uid; - gid_t saved_gid; + if (!pri) { + return; + } + if (conf.use_syslog) + vsyslog(pri, fmt, args); + else if (pri <= conf.log_max_pri) { + FILE *file = logfile ? logfile : stderr; + time_t now = time(NULL); + struct tm *t = localtime(&now); + gchar buf[24]; - saved_gid = setegid(conf.mail_gid); - saved_uid = seteuid(conf.mail_uid); + strftime(buf, 24, "%Y-%m-%d %H:%M:%S", t); + fprintf(file, "%s [%d] ", buf, getpid()); - strftime(buf, 24, "%Y-%m-%d %H:%M:%S", t); - fprintf(file, "%s [%d] ", buf, getpid()); - - vfprintf(file, fmt, args); - fflush(file); - - seteuid(saved_uid); - setegid(saved_gid); - } - } + vfprintf(file, fmt, args); + fflush(file); } } diff -r f899ca0835a8 -r e507c854a63e src/masqmail.c --- a/src/masqmail.c Wed Jul 13 10:30:52 2011 +0200 +++ b/src/masqmail.c Sat Aug 27 16:19:07 2011 +0200 @@ -61,8 +61,10 @@ sigterm_in_progress = 1; if (pidfile) { - uid_t uid; - uid = seteuid(0); + uid_t uid = geteuid(); + if (seteuid(0) != 0) { + logwrite(LOG_ALERT, "sigterm_handler: could not set euid to %d: %s\n", 0, strerror(errno)); + } if (unlink(pidfile) != 0) logwrite(LOG_WARNING, "could not delete pid file %s: %s\n", pidfile, strerror(errno)); seteuid(uid); /* we exit anyway after this, just to be sure */ @@ -187,8 +189,7 @@ conf.do_verbose = FALSE; if (!conf.run_as_user) { - seteuid(conf.orig_uid); - setegid(conf.orig_gid); + set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL); } DEBUG(5) debugf("accepting smtp message on stdin\n"); @@ -217,8 +218,7 @@ } if (!conf.run_as_user) { - seteuid(conf.orig_uid); - setegid(conf.orig_gid); + set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL); } DEBUG(5) debugf("accepting message on stdin\n"); @@ -647,10 +647,15 @@ */ if ((strcmp(conf_file, CONF_FILE) != 0) && (conf.orig_uid != 0)) { conf.run_as_user = TRUE; - seteuid(conf.orig_uid); - setegid(conf.orig_gid); - setuid(conf.orig_uid); - setgid(conf.orig_gid); + set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL); + if (setgid(conf.orig_gid)) { + logwrite(LOG_ALERT, "could not set gid to %d: %s\n", conf.orig_gid, strerror(errno)); + exit(1); + } + if (setuid(conf.orig_uid)) { + logwrite(LOG_ALERT, "could not set uid to %d: %s\n", conf.orig_uid, strerror(errno)); + exit(1); + } } conf.log_dir = LOG_DIR;