pkgsrc/mail/spamass-milter/patches/patch-aa
gdt c7c4d6f66d Memory allocation hygiene fix from PR pkg/44704.
spamass-milter forks, allocates memory and then execs, violating
locking rules.  This commit adds a patch from Juergen Hannken-Illjes
that moves the allocation above the fork.

TODO: upstream is recently alive again, after 5 years with no
releases.  Push these fixes upstream.
2011-03-18 15:26:30 +00:00

350 lines
10 KiB
Text

$NetBSD: patch-aa,v 1.5 2011/03/18 15:26:30 gdt Exp $
This patch has hunks for multiple reasons:
1) Ancient fix to avoid going beyond s2.
2) Added CVE-2010-1132 patch from:
https://bugzilla.redhat.com/attachment.cgi?id=401011
3) (Most of, some in .h) patch to add option to not scan mail from
authenticated users, from:
http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2006-November/106024.html
4) Avoid memory allocation in after fork and before exec. From PR pkg/44704.
--- spamass-milter.cpp.orig 2011-03-18 15:15:56.000000000 +0000
+++ spamass-milter.cpp
@@ -170,10 +170,7 @@ char *spambucket;
bool flag_full_email = false; /* pass full email address to spamc */
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
+bool auth = false; /* don't scan authenticated users */
// {{{ main()
@@ -181,7 +178,7 @@ int
main(int argc, char* argv[])
{
int c, err = 0;
- const char *args = "fd:mMp:P:r:u:D:i:b:B:e:x";
+ const char *args = "fd:mMp:P:r:u:D:i:b:B:e:xa";
char *sock = NULL;
bool dofork = false;
char *pidfilename = NULL;
@@ -196,6 +193,9 @@ main(int argc, char* argv[])
/* Process command line options */
while ((c = getopt(argc, argv, args)) != -1) {
switch (c) {
+ case 'a':
+ auth = true;
+ break;
case 'f':
dofork = true;
break;
@@ -281,7 +281,7 @@ main(int argc, char* argv[])
cout << "SpamAssassin Sendmail Milter Plugin" << endl;
cout << "Usage: spamass-milter -p socket [-b|-B bucket] [-d xx[,yy...]] [-D host]" << endl;
cout << " [-e defaultdomain] [-f] [-i networks] [-m] [-M]" << endl;
- cout << " [-P pidfile] [-r nn] [-u defaultuser] [-x]" << endl;
+ cout << " [-P pidfile] [-r nn] [-u defaultuser] [-x] [-a]" << endl;
cout << " [-- spamc args ]" << endl;
cout << " -p socket: path to create socket" << endl;
cout << " -b bucket: redirect spam to this mail address. The orignal" << endl;
@@ -302,6 +302,7 @@ main(int argc, char* argv[])
cout << " -u defaultuser: pass the recipient's username to spamc.\n"
" Uses 'defaultuser' if there are multiple recipients." << endl;
cout << " -x: pass email address through alias and virtusertable expansion." << endl;
+ cout << " -a: don't scan messages over an authenticated connection." << endl;
cout << " -- spamc args: pass the remaining flags to spamc." << endl;
exit(EX_USAGE);
@@ -461,59 +462,24 @@ 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
- char *fmt="%s \"%s\"";
+ char *popen_argv[3];
FILE *p;
-#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");
+ popen_argv[0] = SENDMAIL;
+ popen_argv[1] = spambucket;
+ popen_argv[2] = NULL;
+
+ debug(D_COPY, "calling %s %s", SENDMAIL, spambucket);
+ p = popenv(popen_argv, "w");
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
fwrite(assassin->d().c_str(), assassin->d().size(), 1, p);
- pclose(p); p = NULL;
+ fclose(p); p = NULL;
}
-#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;
}
@@ -783,6 +749,15 @@ mlfi_envfrom(SMFICTX* ctx, char** envfro
}
/* debug(D_ALWAYS, "ZZZ got private context %p", sctx); */
+ if (auth) {
+ const char *auth_type = smfi_getsymval(ctx, "{auth_type}");
+
+ if (auth_type) {
+ debug(D_MISC, "auth_type=%s", auth_type);
+ return SMFIS_ACCEPT;
+ }
+ }
+
debug(D_FUNC, "mlfi_envfrom: enter");
try {
// launch new SpamAssassin
@@ -842,30 +817,19 @@ 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";
-
-#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 *popen_argv[4];
+
+ popen_argv[0] = SENDMAIL;
+ popen_argv[1] = "-bv";
+ popen_argv[2] = envrcpt[0];
+ popen_argv[3] = NULL;
- 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");
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
{
@@ -890,16 +854,8 @@ mlfi_envrcpt(SMFICTX* ctx, char** envrcp
assassin->expandedrcpt.push_back(p+7);
}
}
- pclose(p); p = NULL;
+ fclose(p); p = NULL;
}
-#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]);
@@ -1343,6 +1299,22 @@ SpamAssassin::~SpamAssassin()
void SpamAssassin::Connect()
{
+ int argc;
+ char *argv[100];
+ char spamc_user[64];
+
+ if (expandedrcpt.size() != 1) {
+ debug(D_RCPT, "%d recipients; spamc gets default username %s", (int)expandedrcpt.size(), defaultuser);
+ strlcpy(spamc_user, defaultuser, sizeof(spamc_user));
+ } else {
+ if (flag_full_email)
+ strlcpy(spamc_user, full_user().c_str(), sizeof(spamc_user));
+ else
+ strlcpy(spamc_user, local_user().c_str(), sizeof(spamc_user));
+ strlwr(spamc_user);
+ debug(D_RCPT, "spamc gets %s", spamc_user);
+ }
+
// set up pipes for in- and output
if (pipe(pipe_io[0]))
throw string(string("pipe error: ")+string(strerror(errno)));
@@ -1376,33 +1348,12 @@ void SpamAssassin::Connect()
// absolute path (determined in autoconf)
// should be a little more secure
// XXX arbitrary 100-argument max
- int argc = 0;
- char** argv = (char**) malloc(100*sizeof(char*));
+ argc = 0;
argv[argc++] = SPAMC;
if (flag_sniffuser)
{
argv[argc++] = "-u";
- if ( expandedrcpt.size() != 1 )
- {
- // More (or less?) than one recipient, so we pass the default
- // username to SPAMC. This way special rules can be defined for
- // multi recipient messages.
- debug(D_RCPT, "%d recipients; spamc gets default username %s", (int)expandedrcpt.size(), defaultuser);
- argv[argc++] = defaultuser;
- } else
- {
- // There is only 1 recipient so we pass the username
- // (converted to lowercase) to SPAMC. Don't worry about
- // freeing this memory as we're exec()ing anyhow.
- if (flag_full_email)
- argv[argc] = strlwr(strdup(full_user().c_str()));
- else
- argv[argc] = strlwr(strdup(local_user().c_str()));
-
- debug(D_RCPT, "spamc gets %s", argv[argc]);
-
- argc++;
- }
+ argv[argc++] = spamc_user;
}
if (spamdhost)
{
@@ -2033,7 +1984,7 @@ cmp_nocase_partial(const string& s, cons
string::const_iterator p=s.begin();
string::const_iterator p2=s2.begin();
- while ( p != s.end() && p2 <= s2.end() ) {
+ while ( p != s.end() ) {
if (toupper(*p) != toupper(*p2))
{
debug(D_STR, "c_nc_p: <%s><%s> : miss", s.c_str(), s2.c_str());
@@ -2157,5 +2108,71 @@ void warnmacro(char *macro, char *scope)
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.
+*/
+FILE *popenv(char *const argv[], const char *type)
+{
+ 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 (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