masqmail-0.2

changeset 184:b3835b6b834b

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.
author markus schnalke <meillo@marmaro.de>
date Sat, 27 Aug 2011 18:00:40 +0200
parents ee2e92148aca
children 1210d3f1af2b
files src/log.c src/masqmail.c
diffstat 2 files changed, 37 insertions(+), 41 deletions(-) [+]
line diff
     1.1 --- a/src/log.c	Fri Jun 03 13:32:14 2011 +0200
     1.2 +++ b/src/log.c	Sat Aug 27 18:00:40 2011 +0200
     1.3 @@ -65,8 +65,9 @@
     1.4  		uid_t saved_uid;
     1.5  		gid_t saved_gid;
     1.6  
     1.7 -		saved_gid = setegid(conf.mail_gid);
     1.8 -		saved_uid = seteuid(conf.mail_uid);
     1.9 +		if (!conf.run_as_user) {
    1.10 +			set_euidgid(conf.mail_uid, conf.mail_gid, &saved_uid, &saved_gid);
    1.11 +		}
    1.12  
    1.13  		filename = g_strdup_printf("%s/masqmail.log", conf.log_dir);
    1.14  		logfile = fopen(filename, "a");
    1.15 @@ -76,8 +77,9 @@
    1.16  		}
    1.17  		g_free(filename);
    1.18  
    1.19 -		seteuid(saved_uid);
    1.20 -		setegid(saved_gid);
    1.21 +		if (!conf.run_as_user) {
    1.22 +			set_euidgid(saved_uid, saved_gid, NULL, NULL);
    1.23 +		}
    1.24  	}
    1.25  
    1.26  #ifdef ENABLE_DEBUG
    1.27 @@ -114,35 +116,26 @@
    1.28  		va_copy(args_copy, args);
    1.29  		vfprintf(stdout, fmt, args_copy);
    1.30  		va_end(args_copy);
    1.31 -		fflush(stdout);  /* is this necessary? */
    1.32 +		fflush(stdout);  /* in case output ends not with newline */
    1.33  	}
    1.34  
    1.35  	pri &= ~LOG_VERBOSE;
    1.36 -	if (pri) {
    1.37 -		if (conf.use_syslog)
    1.38 -			vsyslog(pri, fmt, args);
    1.39 -		else {
    1.40 -			if (pri <= conf.log_max_pri) {
    1.41 -				FILE *file = logfile ? logfile : stderr;
    1.42 -				time_t now = time(NULL);
    1.43 -				struct tm *t = localtime(&now);
    1.44 -				gchar buf[24];
    1.45 -				uid_t saved_uid;
    1.46 -				gid_t saved_gid;
    1.47 +	if (!pri) {
    1.48 +		return;
    1.49 +	}
    1.50 +	if (conf.use_syslog)
    1.51 +		vsyslog(pri, fmt, args);
    1.52 +	else if (pri <= conf.log_max_pri) {
    1.53 +		FILE *file = logfile ? logfile : stderr;
    1.54 +		time_t now = time(NULL);
    1.55 +		struct tm *t = localtime(&now);
    1.56 +		gchar buf[24];
    1.57  
    1.58 -				saved_gid = setegid(conf.mail_gid);
    1.59 -				saved_uid = seteuid(conf.mail_uid);
    1.60 +		strftime(buf, 24, "%Y-%m-%d %H:%M:%S", t);
    1.61 +		fprintf(file, "%s [%d] ", buf, getpid());
    1.62  
    1.63 -				strftime(buf, 24, "%Y-%m-%d %H:%M:%S", t);
    1.64 -				fprintf(file, "%s [%d] ", buf, getpid());
    1.65 -
    1.66 -				vfprintf(file, fmt, args);
    1.67 -				fflush(file);
    1.68 -
    1.69 -				seteuid(saved_uid);
    1.70 -				setegid(saved_gid);
    1.71 -			}
    1.72 -		}
    1.73 +		vfprintf(file, fmt, args);
    1.74 +		fflush(file);
    1.75  	}
    1.76  }
    1.77  
     2.1 --- a/src/masqmail.c	Fri Jun 03 13:32:14 2011 +0200
     2.2 +++ b/src/masqmail.c	Sat Aug 27 18:00:40 2011 +0200
     2.3 @@ -62,8 +62,10 @@
     2.4  	sigterm_in_progress = 1;
     2.5  
     2.6  	if (pidfile) {
     2.7 -		uid_t uid;
     2.8 -		uid = seteuid(0);
     2.9 +		uid_t uid = geteuid();
    2.10 +		if (seteuid(0) != 0) {
    2.11 +			logwrite(LOG_ALERT, "sigterm_handler: could not set euid to %d: %s\n", 0, strerror(errno));
    2.12 +		}
    2.13  		if (unlink(pidfile) != 0)
    2.14  			logwrite(LOG_WARNING, "could not delete pid file %s: %s\n", pidfile, strerror(errno));
    2.15  		seteuid(uid);  /* we exit anyway after this, just to be sure */
    2.16 @@ -236,8 +238,7 @@
    2.17  	conf.do_verbose = FALSE;
    2.18  
    2.19  	if (!conf.run_as_user) {
    2.20 -		seteuid(conf.orig_uid);
    2.21 -		setegid(conf.orig_gid);
    2.22 +		set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL);
    2.23  	}
    2.24  
    2.25  	DEBUG(5) debugf("accepting smtp message on stdin\n");
    2.26 @@ -265,8 +266,7 @@
    2.27  	}
    2.28  
    2.29  	if (!conf.run_as_user) {
    2.30 -		seteuid(conf.orig_uid);
    2.31 -		setegid(conf.orig_gid);
    2.32 +		set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL);
    2.33  	}
    2.34  
    2.35  	DEBUG(5) debugf("accepting message on stdin\n");
    2.36 @@ -632,13 +632,16 @@
    2.37  	   So it is possible for a user to run his own daemon without
    2.38  	   breaking security.
    2.39  	 */
    2.40 -	if (strcmp(conf_file, CONF_FILE) != 0) {
    2.41 -		if (conf.orig_uid != 0) {
    2.42 -			conf.run_as_user = TRUE;
    2.43 -			seteuid(conf.orig_uid);
    2.44 -			setegid(conf.orig_gid);
    2.45 -			setuid(conf.orig_uid);
    2.46 -			setgid(conf.orig_gid);
    2.47 +	if ((strcmp(conf_file, CONF_FILE) != 0) && (conf.orig_uid != 0)) {
    2.48 +		conf.run_as_user = TRUE;
    2.49 +		set_euidgid(conf.orig_uid, conf.orig_gid, NULL, NULL);
    2.50 +		if (setgid(conf.orig_gid)) {
    2.51 +			logwrite(LOG_ALERT, "could not set gid to %d: %s\n", conf.orig_gid, strerror(errno));
    2.52 +			exit(1);
    2.53 +		}
    2.54 +		if (setuid(conf.orig_uid)) {
    2.55 +			logwrite(LOG_ALERT, "could not set uid to %d: %s\n", conf.orig_uid, strerror(errno));
    2.56 +			exit(1);
    2.57  		}
    2.58  	}
    2.59