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.
This commit is contained in:
gdt 2011-03-18 15:26:30 +00:00
parent 20bbbeead8
commit c7c4d6f66d
3 changed files with 74 additions and 13 deletions

View file

@ -1,8 +1,8 @@
# $NetBSD: Makefile,v 1.30 2010/06/10 12:30:21 gdt Exp $
# $NetBSD: Makefile,v 1.31 2011/03/18 15:26:30 gdt Exp $
#
DISTNAME= spamass-milter-0.3.1
PKGREVISION= 3
PKGREVISION= 4
CATEGORIES= mail
MASTER_SITES= http://savannah.nongnu.org/download/spamass-milt/

View file

@ -1,4 +1,4 @@
$NetBSD: distinfo,v 1.10 2010/09/10 23:33:42 gdt Exp $
$NetBSD: distinfo,v 1.11 2011/03/18 15:26:30 gdt Exp $
SHA1 (spamass-milter-0.3.1.tar.gz) = dd488eb9ab1f230440fba8a729bee80550f2fbff
RMD160 (spamass-milter-0.3.1.tar.gz) = 5db6af6b31de1bf83eafbd9713d81cdc957b5033
@ -6,6 +6,6 @@ Size (spamass-milter-0.3.1.tar.gz) = 141144 bytes
SHA1 (spamass-milter-001.patch) = d37227f95808479dc4d6ba5c76ddd2413b4530d3
RMD160 (spamass-milter-001.patch) = eef17cb4506e6f5c0908b6872b7fb5dcd8bc2e16
Size (spamass-milter-001.patch) = 2435 bytes
SHA1 (patch-aa) = 13ba0413c28d14cd1a18d42a0b09ca26b358d913
SHA1 (patch-aa) = f5fd2951082c916e3cae1746f8921793ff09b567
SHA1 (patch-ab) = 03f7d4abc24e950fd44a4adbb708f3433d111643
SHA1 (patch-ac) = 851cbceab64b1a391cfe0aad0ba5a86c88218eb0

View file

@ -1,6 +1,6 @@
$NetBSD: patch-aa,v 1.4 2010/09/10 23:33:42 gdt Exp $
$NetBSD: patch-aa,v 1.5 2011/03/18 15:26:30 gdt Exp $
This patch has hunks for three separate reasons:
This patch has hunks for multiple reasons:
1) Ancient fix to avoid going beyond s2.
@ -13,7 +13,9 @@ authenticated users, from:
http://lists.freebsd.org/pipermail/freebsd-ports-bugs/2006-November/106024.html
--- spamass-milter.cpp.orig 2010-09-10 15:50:58.000000000 +0000
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 */
@ -160,8 +162,6 @@ authenticated users, from:
- /* XXX possible buffer overflow here */
- sprintf(buf, fmt, SENDMAIL, envrcpt[0]);
-#endif
-
- debug(D_RCPT, "calling %s", buf);
+ char *popen_argv[4];
+
+ popen_argv[0] = SENDMAIL;
@ -169,6 +169,9 @@ authenticated users, from:
+ 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)
@ -177,8 +180,7 @@ authenticated users, from:
- abort();
- }
-#endif
+ debug(D_RCPT, "calling %s -bv %s", SENDMAIL, envrcpt[0]);
-
- p = popen(buf, "r");
+ p = popenv(popen_argv, "r");
if (!p)
@ -206,7 +208,66 @@ authenticated users, from:
} else
{
assassin->expandedrcpt.push_back(envrcpt[0]);
@@ -2033,7 +1989,7 @@ cmp_nocase_partial(const string& s, cons
@@ -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();
@ -215,7 +276,7 @@ authenticated users, from:
if (toupper(*p) != toupper(*p2))
{
debug(D_STR, "c_nc_p: <%s><%s> : miss", s.c_str(), s2.c_str());
@@ -2157,5 +2113,71 @@ void warnmacro(char *macro, char *scope)
@@ -2157,5 +2108,71 @@ void warnmacro(char *macro, char *scope)
warnedmacro = true;
}