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