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 <d...@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 <d...@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;
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.
Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
