spamass-milter-0.3.1_9 leaving open zombie processes.
Ted Hatfield
ted at pat.io.com
Mon May 10 19:46:19 UTC 2010
spamass-milter-0.3.0_9 appears to be an update to fix the security
vulnerability referenced by CVE-2010-1132.
However the patch installed for this vulnerability fails to close
processes properly and spamass-milter leaves a large number of zombie
processes open until the milter is restarted.
Rather than wait for the port maintainer to update this port we
installed the patches found at http://savannah.nongnu.org/bugs/?29326
Specifically
file #20020: spamass-milter-0.3.1-syntax.patch
file #20284: spamass-milter-0.3.1-popen.patch
If anyone wants to see tham I have included the patches I used.
Does anyone have an ETA for an official update.
Thank,
Ted Hatfield
PrismNet Ltd.
IO.COM.
-------------- next part --------------
--- spamass-milter.cpp 2006-03-23 21:41:36.000000000 +0000
+++ spamass-milter.cpp 2010-03-23 16:44:54.570023100 +0000
@@ -129,9 +129,11 @@ int daemon(int nochdir, int noclose);
static const char Id[] = "$Id: spamass-milter.cpp,v 1.90 2006/03/23 21:41:36 dnelson Exp $";
+static char FilterName[] = "SpamAssassin";
+
struct smfiDesc smfilter =
{
- "SpamAssassin", // filter name
+ FilterName, // filter name
SMFI_VERSION, // version code -- leave untouched
SMFIF_ADDHDRS|SMFIF_CHGHDRS|SMFIF_CHGBODY, // flags
mlfi_connect, // info filter callback
@@ -361,7 +363,7 @@ main(int argc, char* argv[])
// }}}
/* Update a header if SA changes it, or add it if it is new. */
-void update_or_insert(SpamAssassin* assassin, SMFICTX* ctx, string oldstring, t_setter setter, char *header )
+void update_or_insert(SpamAssassin* assassin, SMFICTX* ctx, string oldstring, t_setter setter, const char *header )
{
string::size_type eoh1 = assassin->d().find("\n\n");
string::size_type eoh2 = assassin->d().find("\n\r\n");
@@ -387,12 +389,12 @@ void update_or_insert(SpamAssassin* assa
if (oldsize > 0)
{
debug(D_UORI, "u_or_i: changing");
- smfi_chgheader(ctx, header, 1, newstring.size() > 0 ?
+ smfi_chgheader(ctx, const_cast<char*>(header), 1, newstring.size() > 0 ?
cstr : NULL );
} else if (newstring.size() > 0)
{
debug(D_UORI, "u_or_i: inserting");
- smfi_addheader(ctx, header, cstr);
+ smfi_addheader(ctx, const_cast<char*>(header), cstr);
}
} else
{
@@ -452,7 +454,7 @@ assassinate(SMFICTX* ctx, SpamAssassin*
if (do_reject)
{
debug(D_MISC, "Rejecting");
- smfi_setreply(ctx, "550", "5.7.1", "Blocked by SpamAssassin");
+ smfi_setreply(ctx, const_cast<char*>("550"), const_cast<char*>("5.7.1"), const_cast<char*>("Blocked by SpamAssassin"));
if (flag_bucket)
@@ -470,7 +472,7 @@ assassinate(SMFICTX* ctx, SpamAssassin*
#else
char buf[1024];
#endif
- char *fmt="%s \"%s\"";
+ const char *fmt="%s \"%s\"";
FILE *p;
#if defined(HAVE_ASPRINTF)
@@ -500,7 +502,10 @@ assassinate(SMFICTX* ctx, SpamAssassin*
} else
{
// Send message provided by SpamAssassin
- fwrite(assassin->d().c_str(), assassin->d().size(), 1, p);
+ if (fwrite(assassin->d().c_str(), assassin->d().size(), 1, p) != 1)
+ {
+ debug(D_COPY, "fwrite incomplete (%s) when copying to spambucket", strerror(errno));
+ }
pclose(p); p = NULL;
}
#if defined(__FreeBSD__)
@@ -531,7 +536,7 @@ assassinate(SMFICTX* ctx, SpamAssassin*
// time. Note, this may generate multiple X-Spam-Orig-To
// headers, but that's okay.
while( !assassin->recipients.empty()) {
- if ( smfi_addheader( ctx, "X-Spam-Orig-To", (char *)assassin->recipients.front().c_str()) != MI_SUCCESS ) {
+ if ( smfi_addheader( ctx, const_cast<char *>("X-Spam-Orig-To"), (char *)assassin->recipients.front().c_str()) != MI_SUCCESS ) {
throw string( "Failed to save recipient" );
}
@@ -774,7 +779,7 @@ mlfi_envfrom(SMFICTX* ctx, char** envfro
{
SpamAssassin* assassin;
struct context *sctx = (struct context *)smfi_getpriv(ctx);
- char *queueid;
+ const char *queueid;
if (sctx == NULL)
{
@@ -801,7 +806,7 @@ mlfi_envfrom(SMFICTX* ctx, char** envfro
// remember the MAIL FROM address
assassin->set_from(string(envfrom[0]));
- queueid=smfi_getsymval(ctx,"i");
+ queueid=smfi_getsymval(ctx, const_cast<char *>("i"));
if (!queueid)
{
queueid="unknown";
@@ -842,7 +847,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
/* open a pipe to sendmail so we can do address expansion */
char buf[1024];
- char *fmt="%s -bv \"%s\" 2>&1";
+ const char *fmt="%s -bv \"%s\" 2>&1";
#if defined(HAVE_SNPRINTF)
snprintf(buf, sizeof(buf)-1, fmt, SENDMAIL, envrcpt[0]);
@@ -928,7 +933,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
char date[32];
/* RFC 822 date. */
- macro_b = smfi_getsymval(ctx, "b");
+ macro_b = smfi_getsymval(ctx, const_cast<char *>("b"));
if (!macro_b)
{
time_t tval;
@@ -939,7 +944,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
}
/* queue ID */
- macro_i = smfi_getsymval(ctx, "i");
+ macro_i = smfi_getsymval(ctx, const_cast<char *>("i"));
if (!macro_i)
{
macro_i = "unknown";
@@ -947,7 +952,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
}
/* FQDN of this site */
- macro_j = smfi_getsymval(ctx, "j");
+ macro_j = smfi_getsymval(ctx, const_cast<char *>("j"));
if (!macro_j)
{
macro_j = "localhost";
@@ -955,7 +960,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
}
/* Protocol used to receive the message */
- macro_r = smfi_getsymval(ctx, "r");
+ macro_r = smfi_getsymval(ctx, const_cast<char *>("r"));
if (!macro_r)
{
macro_r = "SMTP";
@@ -967,14 +972,14 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
fixed. Until that day, use the value remembered by
mlfi_helo()
*/
- macro_s = smfi_getsymval(ctx, "s");
+ macro_s = smfi_getsymval(ctx, const_cast<char *>("s"));
if (!macro_s)
macro_s = sctx->helo;
if (!macro_s)
macro_s = "nohelo";
/* Sendmail binary version */
- macro_v = smfi_getsymval(ctx, "v");
+ macro_v = smfi_getsymval(ctx, const_cast<char *>("v"));
if (!macro_v)
{
macro_v = "8.13.0";
@@ -982,7 +987,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
}
/* Sendmail .cf version */
- macro_Z = smfi_getsymval(ctx, "Z");
+ macro_Z = smfi_getsymval(ctx, const_cast<char *>("Z"));
if (!macro_Z)
{
macro_Z = "8.13.0";
@@ -990,7 +995,7 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
}
/* Validated sending site's address */
- macro__ = smfi_getsymval(ctx, "_");
+ macro__ = smfi_getsymval(ctx, const_cast<char *>("_"));
if (!macro__)
{
macro__ = "unknown";
@@ -1378,10 +1383,10 @@ void SpamAssassin::Connect()
// XXX arbitrary 100-argument max
int argc = 0;
char** argv = (char**) malloc(100*sizeof(char*));
- argv[argc++] = SPAMC;
+ argv[argc++] = strdup(SPAMC);
if (flag_sniffuser)
{
- argv[argc++] = "-u";
+ argv[argc++] = strdup("-u");
if ( expandedrcpt.size() != 1 )
{
// More (or less?) than one recipient, so we pass the default
@@ -1406,7 +1411,7 @@ void SpamAssassin::Connect()
}
if (spamdhost)
{
- argv[argc++] = "-d";
+ argv[argc++] = strdup("-d");
argv[argc++] = spamdhost;
}
if (spamc_argc)
@@ -2148,7 +2153,7 @@ char *strlwr(char *str)
}
/* Log a message about missing milter macros, but only the first time */
-void warnmacro(char *macro, char *scope)
+void warnmacro(const char *macro, const char *scope)
{
if (warnedmacro)
return;
--- spamass-milter.cpp 2010-04-19 11:47:57.369162724 +0100
+++ spamass-milter.cpp 2010-04-19 11:50:21.404162719 +0100
@@ -173,10 +173,6 @@ bool flag_full_email = false; /* pass f
bool flag_expand = false; /* alias/virtusertable expansion */
bool warnedmacro = false; /* have we logged that we couldn't fetch a macro? */
-#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
-static pthread_mutex_t popen_mutex = PTHREAD_MUTEX_INITIALIZER;
-#endif
-
// {{{ main()
int
@@ -463,42 +459,16 @@ assassinate(SMFICTX* ctx, SpamAssassin*
send another copy. The milter API will not let you send the
message AND return a failure code to the sender, so this is
the only way to do it. */
-#if defined(__FreeBSD__)
- int rv;
-#endif
-
-#if defined(HAVE_ASPRINTF)
- char *buf;
-#else
- char buf[1024];
-#endif
- const char *fmt="%s \"%s\"";
+ char sendmail_prog[] = SENDMAIL;
+ char *const popen_argv[] = { sendmail_prog, spambucket, NULL };
FILE *p;
+ pid_t pid;
-#if defined(HAVE_ASPRINTF)
- asprintf(&buf, fmt, SENDMAIL, spambucket);
-#else
-#if defined(HAVE_SNPRINTF)
- snprintf(buf, sizeof(buf)-1, fmt, SENDMAIL, spambucket);
-#else
- /* XXX possible buffer overflow here */
- sprintf(buf, fmt, SENDMAIL, spambucket);
-#endif
-#endif
-
- debug(D_COPY, "calling %s", buf);
-#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
- rv = pthread_mutex_lock(&popen_mutex);
- if (rv)
- {
- debug(D_ALWAYS, "Could not lock popen mutex: %s", strerror(rv));
- abort();
- }
-#endif
- p = popen(buf, "w");
+ debug(D_COPY, "calling %s %s", SENDMAIL, spambucket);
+ p = popenv(popen_argv, "w", &pid);
if (!p)
{
- debug(D_COPY, "popen failed(%s). Will not send a copy to spambucket", strerror(errno));
+ debug(D_COPY, "popenv failed(%s). Will not send a copy to spambucket", strerror(errno));
} else
{
// Send message provided by SpamAssassin
@@ -506,19 +476,9 @@ assassinate(SMFICTX* ctx, SpamAssassin*
{
debug(D_COPY, "fwrite incomplete (%s) when copying to spambucket", strerror(errno));
}
- pclose(p); p = NULL;
+ fclose(p); p = NULL;
+ waitpid(pid, NULL, 0);
}
-#if defined(__FreeBSD__)
- rv = pthread_mutex_unlock(&popen_mutex);
- if (rv)
- {
- debug(D_ALWAYS, "Could not unlock popen mutex: %s", strerror(rv));
- abort();
- }
-#endif
-#if defined(HAVE_ASPRINTF)
- free(buf);
-#endif
}
return SMFIS_REJECT;
}
@@ -847,30 +807,17 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
/* open a pipe to sendmail so we can do address expansion */
char buf[1024];
- const char *fmt="%s -bv \"%s\" 2>&1";
-
-#if defined(HAVE_SNPRINTF)
- snprintf(buf, sizeof(buf)-1, fmt, SENDMAIL, envrcpt[0]);
-#else
- /* XXX possible buffer overflow here */
- sprintf(buf, fmt, SENDMAIL, envrcpt[0]);
-#endif
+ char sendmail_prog[] = SENDMAIL;
+ char sendmail_mode[] = "-bv";
+ char * const popen_argv[] = { sendmail_prog, sendmail_mode, envrcpt[0], NULL };
+ pid_t pid;
- debug(D_RCPT, "calling %s", buf);
+ debug(D_RCPT, "calling %s -bv %s", SENDMAIL, envrcpt[0]);
-#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
- rv = pthread_mutex_lock(&popen_mutex);
- if (rv)
- {
- debug(D_ALWAYS, "Could not lock popen mutex: %s", strerror(rv));
- abort();
- }
-#endif
-
- p = popen(buf, "r");
+ p = popenv(popen_argv, "r", &pid);
if (!p)
{
- debug(D_RCPT, "popen failed(%s). Will not expand aliases", strerror(errno));
+ debug(D_RCPT, "popenv failed(%s). Will not expand aliases", strerror(errno));
assassin->expandedrcpt.push_back(envrcpt[0]);
} else
{
@@ -895,16 +842,9 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
assassin->expandedrcpt.push_back(p+7);
}
}
- pclose(p); p = NULL;
+ fclose(p); p = NULL;
+ waitpid(pid, NULL, 0);
}
-#if defined(__FreeBSD__)
- rv = pthread_mutex_unlock(&popen_mutex);
- if (rv)
- {
- debug(D_ALWAYS, "Could not unlock popen mutex: %s", strerror(rv));
- abort();
- }
-#endif
} else
{
assassin->expandedrcpt.push_back(envrcpt[0]);
@@ -2162,5 +2102,72 @@ void warnmacro(const char *macro, const
warnedmacro = true;
}
+/*
+ untrusted-argument-safe popen function - only supports "r" and "w" modes
+ for simplicity, and always reads stdout and stderr in "r" mode. Call
+ fclose to close the FILE, and waitpid to reap the child process (pid).
+*/
+FILE *popenv(char *const argv[], const char *type, pid_t *pid)
+{
+ FILE *iop;
+ int pdes[2];
+ int save_errno;
+
+ if ((*type != 'r' && *type != 'w') || type[1])
+ {
+ errno = EINVAL;
+ return (NULL);
+ }
+ if (pipe(pdes) < 0)
+ return (NULL);
+ switch (*pid = fork()) {
+
+ case -1: /* Error. */
+ save_errno = errno;
+ (void)close(pdes[0]);
+ (void)close(pdes[1]);
+ errno = save_errno;
+ return (NULL);
+ /* NOTREACHED */
+ case 0: /* Child. */
+ if (*type == 'r') {
+ /*
+ * The dup2() to STDIN_FILENO is repeated to avoid
+ * writing to pdes[1], which might corrupt the
+ * parent's copy. This isn't good enough in
+ * general, since the exit() is no return, so
+ * the compiler is free to corrupt all the local
+ * variables.
+ */
+ (void)close(pdes[0]);
+ (void)dup2(pdes[1], STDOUT_FILENO);
+ (void)dup2(pdes[1], STDERR_FILENO);
+ if (pdes[1] != STDOUT_FILENO && pdes[1] != STDERR_FILENO) {
+ (void)close(pdes[1]);
+ }
+ } else {
+ if (pdes[0] != STDIN_FILENO) {
+ (void)dup2(pdes[0], STDIN_FILENO);
+ (void)close(pdes[0]);
+ }
+ (void)close(pdes[1]);
+ }
+ execv(argv[0], argv);
+ exit(127);
+ /* NOTREACHED */
+ }
+
+ /* Parent; assume fdopen can't fail. */
+ if (*type == 'r') {
+ iop = fdopen(pdes[0], type);
+ (void)close(pdes[1]);
+ } else {
+ iop = fdopen(pdes[1], type);
+ (void)close(pdes[0]);
+ }
+
+ return (iop);
+}
+
// }}}
// vim6:ai:noexpandtab
-------------- next part --------------
--- spamass-milter.h 2006-03-23 22:07:55.000000000 +0000
+++ spamass-milter.h 2010-03-23 16:29:58.281863158 +0000
@@ -185,6 +185,6 @@ void parse_networklist(char *string, str
int ip_in_networklist(struct in_addr ip, struct networklist *list);
void parse_debuglevel(char* string);
char *strlwr(char *str);
-void warnmacro(char *macro, char *scope);
+void warnmacro(const char *macro, const char *scope);
#endif
--- spamass-milter.h 2010-04-19 11:47:57.403162755 +0100
+++ spamass-milter.h 2010-04-19 11:48:32.588162181 +0100
@@ -186,5 +186,6 @@ int ip_in_networklist(struct in_addr ip,
void parse_debuglevel(char* string);
char *strlwr(char *str);
void warnmacro(const char *macro, const char *scope);
+FILE *popenv(char *const argv[], const char *type, pid_t *pid);
#endif
More information about the freebsd-ports
mailing list