Bug#527862: libmilter: Remote DoS? All milters on multiple servers segfaulting

[Bjørn Mork]

tags 527862 + patch security
thanks

The last few days I've had both clamav-milter and spamass-milter segfault on
two separate servers both running Debian lenny:

Jul 13 04:59:53 canardo kernel: [9021793.803024] spamass-milter[22767]: 
segfault at 130 ip 7f94da384900 sp 429190e0 error 4 in 
libmilter.so.1.0.1[7f94da379000+f000]
Jul 13 05:00:33 canardo kernel: [9021863.827618] clamav-milter[22887]: segfault 
at 130 ip 7f7aaa945900 sp 4234b0f0 error 4 in 
libmilter.so.1.0.1[7f7aaa93a000+f000]

Jul 13 05:55:57 huey kernel: [4728098.560126] spamass-milter[20935]: segfault 
at 130 ip 7fa1b92d1900 sp 4178a0e0 error 4 in 
libmilter.so.1.0.1[7fa1b92c6000+f000]

This might be because they are handling mail for some of the same
domains, and that a single buggy mail server is bringing them both down.
But I believe it is still a somewhat serious security issue, as it is
obviously possible to bring down virus and spam filtering by a remote
connection.

I'm now testing a modified version of the patch attached to this bug,
and this seems to fix the problem.  Please consider adding this to Lenny
as a security fix.  Thanks.

Note that I suspect that the patch has a bug which prevents it from
working (all milters would just hang with the original patch):

> -             int nfd, rfd, i;
> +             int nfd = 0, rfd, i;

I don't think setting nfd = 0 at this point makes sense. If we hit

                /* timeout */
                if (rfd == 0)
                        continue;

then the loop will continue with rebuild_set == false and nfd == 0.

I'm attaching a new patch where this is fixed.



Bjørn


--- ./libmilter/worker.c.bak	2009-07-13 17:41:07.000000000 +0200
+++ ./libmilter/worker.c	2009-07-13 17:47:31.000000000 +0200
@@ -328,6 +328,7 @@
 	int dim_pfd = 0;
 	bool rebuild_set = true;
 	int pcnt = 0; /* error count for poll() failures */
+	time_t lastcheck;
 
 	Tskmgr.tm_tid = sthread_get_id();
 	if (pthread_detach(Tskmgr.tm_tid) != 0)
@@ -345,12 +346,12 @@
 	}
 	dim_pfd = PFD_STEP;
 
+	lastcheck = time(NULL);
 	for (;;)
 	{
 		SMFICTX_PTR ctx;
 		int nfd, rfd, i;
 		time_t now;
-		time_t lastcheck;
 
 		POOL_LEV_DPRINTF(4, ("Let's %s again...", WAITFN));
 
@@ -364,20 +365,20 @@
 		/* check for timed out sessions? */
 		if (lastcheck + DT_CHECK_OLD_SESSIONS < now)
 		{
-			SM_TAILQ_FOREACH(ctx, &WRK_CTX_HEAD, ctx_link)
+			ctx = SM_TAILQ_FIRST(&WRK_CTX_HEAD);
+			while (ctx != SM_TAILQ_END(&WRK_CTX_HEAD))
 			{
+				SMFICTX_PTR ctx_nxt;
+
+				ctx_nxt = SM_TAILQ_NEXT(ctx, ctx_link);
 				if (ctx->ctx_wstate == WKST_WAITING)
 				{
 					if (ctx->ctx_wait == 0)
-					{
 						ctx->ctx_wait = now;
-						continue;
-					}
-
-					/* if session timed out, close it */
-					if (ctx->ctx_wait + OLD_SESSION_TIMEOUT
-					    < now)
+					else if (ctx->ctx_wait + OLD_SESSION_TIMEOUT
+						 < now)
 					{
+						/* if session timed out, close it */
 						sfsistat (*fi_close) __P((SMFICTX *));
 
 						POOL_LEV_DPRINTF(4,
@@ -389,10 +390,9 @@
 							(void) (*fi_close)(ctx);
 
 						mi_close_session(ctx);
-						ctx = SM_TAILQ_FIRST(&WRK_CTX_HEAD);
-						continue;
 					}
 				}
+				ctx = ctx_nxt;
 			}
 			lastcheck = now;
 		}
@@ -465,6 +465,7 @@
 					}
 				}
 			}
+			rebuild_set = false;
 		}
 
 		TASKMGR_UNLOCK();