[Mimedefang] Privilege escalation via PID file manipulation

[Dianne Skoll]

Here's a patch that should apply against MIMEDefang 2.80.

Again, I cannot see any way to completely close this hole as long as
we're holding an fcnrtl(F_SETLCK)-style lock, since the descriptor
must be kept open.  I do as much as I can to mitigate this by
destroying the variable containing the fd, but an attacker could
pretty quickly discover which fd is pointing to the lock file.

Since an exploit requires compromising the daemon, I would say this
is not a super-urgent problem.

Regards,

Dianne.

diff --git a/Changelog b/Changelog
index da1a867..d056e4f 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,11 @@ WARNING: Before upgrading MIMEDefang, please search this file for
 *** NOTE INCOMPATIBILITY ** to see if anything has changed that might
 affect your filter.
 
+2017-08-31 Dianne Skoll <dfs at roaringpenguin.com>
+
+	* Make mimedefang and mimedefang-multiplexor write their PID files
+	as root to avoid an unprivileged user tampering with the pidfiles.
+
 2017-07-24 Dianne Skoll <dfs at roaringpenguin.com>
 
 	* MIMEDefang 2.80 RELEASED
diff --git a/mimedefang-multiplexor.c b/mimedefang-multiplexor.c
index 13b77b9..3dbf6e0 100644
--- a/mimedefang-multiplexor.c
+++ b/mimedefang-multiplexor.c
@@ -566,6 +566,7 @@ main(int argc, char *argv[], char **env)
     int c;
     int n;
     char *pidfile = NULL;
+    int pidfile_fd = -1;
     char *user = NULL;
     char *options;
     int facility = LOG_MAIL;
@@ -919,6 +920,17 @@ main(int argc, char *argv[], char **env)
 	    strcat((char *) Settings.sockName, "/mimedefang-multiplexor.sock");
     }
 
+    /* Open the pidfile as root.  We'll lock it later in the grandchild */
+    if (pidfile) {
+	pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+	if (pidfile_fd < 0) {
+	    syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+	    exit(EXIT_FAILURE);
+	}
+	/* It needs to be world-readable */
+	fchmod(pidfile_fd, 0644);
+    }
+
     /* Drop privileges */
     if (user) {
 	pw = getpwnam(user);
@@ -1134,7 +1146,7 @@ main(int argc, char *argv[], char **env)
     }
 
     for (i=0; i<256; i++) {
-	if (i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
+	if (i == pidfile_fd || i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
 	(void) close(i);
     }
 
@@ -1155,10 +1167,12 @@ main(int argc, char *argv[], char **env)
 
     /* Write pid */
     if (pidfile) {
-	if (write_and_lock_pidfile(pidfile) < 0) {
+	if (write_and_lock_pidfile(pidfile, pidfile_fd) < 0) {
 	    exit(1);
 	}
 	free(pidfile);
+	/* Forget the fd */
+	pidfile_fd = -1;
     }
 
     /* Set up SIGHUP handler */
diff --git a/mimedefang.c b/mimedefang.c
index 7e74137..5d545c4 100644
--- a/mimedefang.c
+++ b/mimedefang.c
@@ -2331,6 +2331,7 @@ main(int argc, char **argv)
     int got_p_option = 0;
     int kidpipe[2];
     char kidmsg[256];
+    int pidfile_fd = -1;
 
     mode_t socket_umask = 077;
     mode_t file_umask   = 077;
@@ -2611,6 +2612,17 @@ main(int argc, char **argv)
 	exit(EXIT_FAILURE);
     }
 
+    /* Open the pidfile as root.  We'll lock it later in the grandchild */
+    if (pidfile) {
+	pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+	if (pidfile_fd < 0) {
+	    syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+	    exit(EXIT_FAILURE);
+	}
+	/* It needs to be world-readable */
+	fchmod(pidfile_fd, 0644);
+    }
+
     /* Look up user */
     if (user) {
 	pw = getpwnam(user);
@@ -2715,12 +2727,14 @@ main(int argc, char **argv)
 
     /* Write pid */
     if (pidfile) {
-	if (write_and_lock_pidfile(pidfile) < 0) {
+	if (write_and_lock_pidfile(pidfile, pidfile_fd) < 0) {
 	    /* Signal the waiting parent */
 	    REPORT_FAILURE("Cannot lock pidfile: Is another copy running?", 45);
 	    exit(1);
 	}
 	free(pidfile);
+	/* Forget the fd */
+	pidfile_fd = -1;
     }
 
     (void) closelog();
diff --git a/mimedefang.h b/mimedefang.h
index 381316d..608c2e6 100644
--- a/mimedefang.h
+++ b/mimedefang.h
@@ -66,7 +66,7 @@ extern int make_listening_socket(char const *str, int backlog, int must_be_unix)
 extern void do_delay(char const *sleepstr);
 extern int is_localhost(struct sockaddr *);
 extern int remove_local_socket(char const *str);
-extern int write_and_lock_pidfile(char const *pidfile);
+extern int write_and_lock_pidfile(char const *pidfile, int fd);
 #ifdef EMBED_PERL
 extern int make_embedded_interpreter(char const *progPath,
 				     char const *subFilter,
diff --git a/utils.c b/utils.c
index 7d4f9c1..1ca3db6 100644
--- a/utils.c
+++ b/utils.c
@@ -1300,9 +1300,8 @@ free_debug(void *ctx, void *x, char const *fname, int line)
 #endif
 
 int
-write_and_lock_pidfile(char const *pidfile)
+write_and_lock_pidfile(char const *pidfile, int fd)
 {
-    int fd;
     struct flock fl;
     char buf[64];
 
@@ -1311,11 +1310,6 @@ write_and_lock_pidfile(char const *pidfile)
     fl.l_start = 0;
     fl.l_len = 0;
 
-    fd = open(pidfile, O_RDWR|O_CREAT, 0666);
-    if (fd < 0) {
-	syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
-	return -1;
-    }
     if (fcntl(fd, F_SETLK, &fl) < 0) {
 	syslog(LOG_ERR, "Could not lock PID file %s: %m.  Is another copy running?", pidfile);
 	return -1;