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.
--- 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
--- 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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ports
To unsubscribe, send any mail to "[email protected]"

Reply via email to