# HG changeset patch
# User markus schnalke <meillo@marmaro.de>
# 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;