Re: [Mimedefang] Privilege escalation via PID file manipulation
On 08/31/2017 04:42 PM, Dianne Skoll wrote: > Hi, > > This is a much more extensive patch, but I believe it does finally > close the hole if you keep your PID files in a root-owned directory. > > Please test this; I plan on releasing 2.81 tomorrow. > I applied the patch and updated the Gentoo init script with the new -p and -o changes, and now everything looks good. The two PID files are located directly in /run and owned by root:root, while the two lock files live in the spool directory and are owned by defang:defang. The daemon starts/stops without issue. Thanks once more for your help with this. I'll ask for a CVE assignment in a moment, and then wait until the new version is released before making an announcement for the distros. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
Hi,
This is a much more extensive patch, but I believe it does finally
close the hole if you keep your PID files in a root-owned directory.
Please test this; I plan on releasing 2.81 tomorrow.
Regards,
Dianne.
diff --git a/Changelog b/Changelog
index da1a867..b9f09fa 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,18 @@ 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
+
+ * Make mimedefang and mimedefang-multiplexor write their PID files
+ as root to avoid an unprivileged user tampering with the pidfiles.
+
+ *** NOTE INCOMPATIBILITY ***
+
+ You should move your PID files out of the MIMEDefang spool directory
+ and into a standard root-owned directory like /var/run. Use the -o
+ option to create lock files in the spool directory. The sample
+ init scripts have been updated to reflect this.
+
2017-07-24 Dianne Skoll
* MIMEDefang 2.80 RELEASED
diff --git a/examples/init-script.in b/examples/init-script.in
index 346efca..8da803f 100755
--- a/examples/init-script.in
+++ b/examples/init-script.in
@@ -10,8 +10,10 @@
RETVAL=0
prog='mimedefang'
SPOOLDIR='@SPOOLDIR@'
-PID="$SPOOLDIR/$prog.pid"
-MXPID="$SPOOLDIR/$prog-multiplexor.pid"
+PID="/var/run/$prog.pid"
+MXPID="/var/run/$prog-multiplexor.pid"
+LOCK="$SPOOLDIR/$prog.lock"
+MXLOCK="$SPOOLDIR/$prog-multiplexor.lock"
# These lines keep SpamAssassin happy. Not needed if you
# aren't using SpamAssassin.
@@ -229,7 +231,7 @@ start_it() {
else
EMBEDFLAG=""
fi
-$PROGDIR/$prog-multiplexor -p $MXPID \
+$PROGDIR/$prog-multiplexor -p $MXPID -o $MXLOCK \
$EMBEDFLAG \
`[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
`[ -n "$FILTER" ] && echo "-f $FILTER"` \
@@ -269,7 +271,7 @@ start_it() {
# Start mimedefang
printf "%-60s" "Starting $prog: "
rm -f $SOCKET > /dev/null 2>&1
-$PROGDIR/$prog -P $PID -R $LOOPBACK_RESERVED_CONNECTIONS \
+$PROGDIR/$prog -P $PID -o $LOCK -R $LOOPBACK_RESERVED_CONNECTIONS \
-m $MX_SOCKET \
`[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
`[ -n "$MX_USER" ] && echo "-U $MX_USER"` \
diff --git a/mimedefang-multiplexor.8.in b/mimedefang-multiplexor.8.in
index 980b57d..3505e48 100644
--- a/mimedefang-multiplexor.8.in
+++ b/mimedefang-multiplexor.8.in
@@ -117,7 +117,18 @@ for the format of \fIsocket\fR.
.TP
.B \-p \fIfileName\fR
Causes \fBmimedefang-multiplexor\fR to write its process-ID (after
-becoming a daemon) to the specified file.
+becoming a daemon) to the specified file. The file will be owned
+by root.
+
+.TP
+.B \-o \fIfileName\fR
+Causes \fbmimedefang-multiplexor\fR to use \fIfileName\fR as a lock
+file to avoid multiple instances from running. If you supply
+\fB\-p\fR but not \fB\-o\fR, then \fbmimedefang-multiplexor\fR
+constructs a lock file by appending ".lock" to the pid file. However,
+this is less secure than having a root-owned pid file in a root-owned
+directory and a lock file writable by the user named by the \fB\-U\fR
+option. (The lock file must be writable by the \fB\-U\fR user.)
.TP
.B \-f \fIfilter_path\fR
diff --git a/mimedefang-multiplexor.c b/mimedefang-multiplexor.c
index 13b77b9..2e44f12 100644
--- a/mimedefang-multiplexor.c
+++ b/mimedefang-multiplexor.c
@@ -56,6 +56,12 @@
static void limit_mem_usage(unsigned long rss, unsigned long as);
#endif
+static char *pidfile = NULL;
+static char *lockfile = NULL;
+
+/* Number of file descriptors to close when forking */
+#define CLOSEFDS 256
+
/* Weird case, but hey... */
#if defined(HAVE_WAIT3) && !defined(HAVE_SETRLIMIT)
#include
@@ -346,6 +352,7 @@ static int get_hourly_history_totals(int cmd, time_t now,
int hours, int *total,
#define NUM_FREE_SLAVES(SlaveCount[STATE_IDLE] + SlaveCount[STATE_STOPPED])
#define NUM_RUNNING_SLAVES (SlaveCount[STATE_IDLE] + SlaveCount[STATE_BUSY] +
SlaveCount[STATE_KILLED])
+#define REPORT_FAILURE(msg) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E"
msg, strlen(msg)+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
/**
* %FUNCTION: state_name
@@ -496,6 +503,7 @@ usage(void)
fprintf(stderr, " -v-- Print version and exit\n");
fprintf(stderr, " -t filename -- Log statistics to filename\n");
fprintf(stderr, " -p filename -- Write process-ID in filename\n");
+fprintf(stderr, " -o file -- Use specified file as a lock
file\n");
fprintf(stderr, " -T-- Log statistics to syslog\n");
fprintf(stderr, " -u-- Flush stats file after each
write\n");
fprintf(stderr, " -Z-- Accept and process status updates
from busy slaves\n");
@@ -565,10 +573,13 @@ main(int argc, char *argv[], char **env)
int sock, unpriv_sock;
int c;
Re: [Mimedefang] Privilege escalation via PID file manipulation
Hi, The patch I posted earlier does not completely fix the problem. True, the pid file is owned by root, but it's created in a directory owned by defang, so there's still a way for the "defang" user to subvert this. I will have a patch by tomorrow that separates out the pid file (which will be root-owned in a root-owned directory) from the lock file (which can be defang-owned in a defang-owned directory.) Regards, Dianne. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
On Thu, 31 Aug 2017 12:11:05 -0400 Michael Orlitzky wrote: > Hmmm, in that case, maybe the PID file is being reused for a purpose > that it isn't really suited for? The contents of the PID file are > slightly sensitive, since init scripts tend to trust them -- but the > contents of a lock file aren't. Would it make more sense to have a > separate lock file, whose only purpose is to prevent multiple daemons > from starting (and not to provide info to an init system)? That makes sense. I'll do it that way. Thanks for alerting me to this. Regards, Dianne. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
On 08/31/2017 11:55 AM, Dianne Skoll wrote: > >> You'll have to forgive the stupid question since I'm not a regular >> user of MIMEDefang, but what's the purpose of the file lock? Is it to >> prevent multiple daemons from running at the same time when they're >> not managed by an init system? > > Yep. In the days of systemd and the like, this is probably not > necessary, but not everyone runs systemd. Hmmm, in that case, maybe the PID file is being reused for a purpose that it isn't really suited for? The contents of the PID file are slightly sensitive, since init scripts tend to trust them -- but the contents of a lock file aren't. Would it make more sense to have a separate lock file, whose only purpose is to prevent multiple daemons from starting (and not to provide info to an init system)? > If people do use systemd or whatever, then they'd start mimedefang and > mimedefang-multiplexor without the options that create the pidfiles > and let systemd manage the processes. Yeah, this is pretty much only an issue for traditional SysV-style init systems, because trying to answer "can I trust the contents of this PID file?" is next to impossible in portable shell script. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
On Thu, 31 Aug 2017 11:38:25 -0400 Michael Orlitzky wrote: > You'll have to forgive the stupid question since I'm not a regular > user of MIMEDefang, but what's the purpose of the file lock? Is it to > prevent multiple daemons from running at the same time when they're > not managed by an init system? Yep. In the days of systemd and the like, this is probably not necessary, but not everyone runs systemd. If people do use systemd or whatever, then they'd start mimedefang and mimedefang-multiplexor without the options that create the pidfiles and let systemd manage the processes. Regards, Dianne. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
On 08/31/2017 11:24 AM, Dianne Skoll wrote: > Here's a patch that should apply against MIMEDefang 2.80. Wow, that was fast, thanks. > 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. You'll have to forgive the stupid question since I'm not a regular user of MIMEDefang, but what's the purpose of the file lock? Is it to prevent multiple daemons from running at the same time when they're not managed by an init system? > Since an exploit requires compromising the daemon, I would say this > is not a super-urgent problem. Agreed there, thanks again for the quick fix. ___ 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
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
+
+ * 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
* 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
Re: [Mimedefang] Privilege escalation via PID file manipulation
Hi, > The MIMEDefang daemons should create their PID files before dropping > privileges. This represents a minor security issue; additional factors > are needed to make it exploitable. I have made a patch to open the PID files as root. However, since the process has to keep the file descriptor open in order not to lose the file lock, it doesn't completely eliminate the chance of an exploit. I will post the patch in a little while, once I have thoroughly tested it. Regards, Dianne. ___ 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
[Mimedefang] Privilege escalation via PID file manipulation
== Summary == The MIMEDefang daemons should create their PID files before dropping privileges. This represents a minor security issue; additional factors are needed to make it exploitable. == Details == The purpose of the PID file is to hold the PID of the running daemon, so that later it can be stopped, restarted, or otherwise signalled (many daemons reload their configurations in response to a SIGHUP). To fulfil that purpose, the contents of the PID file need to be trustworthy. If the PID file is writable by a non-root user, then he can replace its contents with the PID of a root process. Afterwards, any attempt to signal the PID contained in the PID file will instead signal a root process chosen by the non-root user (a vulnerability). This is commonly exploitable through init scripts that are run as root and which blindly trust the contents of their PID files. Examples of said init scripts can be found in the MIMEDefang source tree: * examples/init-script.in * redhat/mimedefang-init.in == Exploitation == An example of a problematic scenario involving an init script would be, 1. I run "/etc/init.d/mimedefang start" to start the daemon. 2. mimedefang drops to the "defang" user. 3. mimedefang writes its PID file, now owned by the "defang" user. 4. Someone compromises the daemon. 5. The attacker is generally limited in what he can do because the daemon doesn't run as root. However, he can write "1" into the PID file, and he does. 6. I run "/etc/init.d/mimedefang stop" to stop the daemon while I investigate the weird behavior resulting from the hack. 7. The machine reboots, because I killed PID 1 (this is normally restricted to root). == Resolution == The problem is usually avoided by creating the PID file as root, before dropping privileges. ___ 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
