Cyrus IMAPd 2.3.10 Released

[Simon Matter]

> Hi,
>
>>>>>> On Sun, 28 Oct 2007 19:42:02 +0100
>>>>>> Tomas Janousek <tjanouse at redhat.com> said:
>
> tjanouse> Looks correct. (will not terminate if it reaches NGROUPS, don't
> know if that
> tjanouse> can happen though)
>
> Oops, it never happen.
> It is intended to be safe-keeping for avoiding endless-loop.
>
> tjanouse> But as I said earlier, it's probably better to use the old code
> on non-glibc
> tjanouse> systems.
>
> I'm not sure which one is better.

The implementation in 2.3.10 also segfaults on Linux systems unpatched
glibc-2.3.2. From the getgrouplist manpage:

BUGS
The glibc 2.3.2 implementation of this function  is  broken:  it 
overwrites memory when the actual number of groups is larger than
*ngroups.

Regards,
Simon

>
> Index: lib/auth_unix.c
> diff -u -p lib/auth_unix.c.orig lib/auth_unix.c
> --- lib/auth_unix.c.orig	2007-09-28 05:02:45.000000000 +0900
> +++ lib/auth_unix.c	2007-10-29 04:23:57.000000000 +0900
> @@ -45,6 +45,9 @@
>   */
>
>  #include <config.h>
> +#ifdef HAVE_SYS_PARAM_H
> +#include <sys/param.h>
> +#endif
>  #include <stdlib.h>
>  #include <pwd.h>
>  #include <grp.h>
> @@ -225,7 +228,12 @@ static struct auth_state *mynewstate(con
>      struct group *grp;
>  #ifdef HAVE_GETGROUPLIST
>      gid_t gid, *groupids = NULL;
> -    int ret, ngroups = 0;
> +    int ret = -1;
> +#ifdef __GLIBC__
> +    int ngroups = 0;
> +#else
> +    int ngroups = 5;
> +#endif
>  #else
>      char **mem;
>  #endif
> @@ -248,22 +256,35 @@ static struct auth_state *mynewstate(con
>  #ifdef HAVE_GETGROUPLIST
>      gid = pwd ? pwd->pw_gid : (gid_t) -1;
>
> +#ifdef __GLIBC__
>      /* get number of groups user is member of into ngroups */
>      getgrouplist(identifier, gid, NULL, &ngroups);
> +#endif
>
>      /* get the actual group ids */
> -    do {
> +    for (;;) {
>  	groupids = (gid_t *)xrealloc((gid_t *)groupids,
>  				     ngroups * sizeof(gid_t));
>
>  	newstate->ngroups = ngroups; /* copy of ngroups for comparision */
>  	ret = getgrouplist(identifier, gid, groupids, &ngroups);
> +	if (ret != -1)
> +	    break;
> +#ifdef __GLIBC__
>  	/*
>  	 * This is tricky. We do this as long as getgrouplist tells us to
>  	 * realloc _and_ the number of groups changes. It tells us to realloc
>  	 * also in the case of failure...
>  	 */
> -    } while (ret != -1 && ngroups != newstate->ngroups);
> +	if (ngroups == newstate->ngroups)
> +	    break;
> +#else
> +	if (newstate->ngroups >= NGROUPS)
> +	    break;
> +	if ((ngroups = newstate->ngroups * 2) > NGROUPS)
> +	    ngroups = NGROUPS;
> +#endif
> +    }
>
>      if (ret == -1) {
>  	newstate->ngroups = 0;
>
>
> Sincerely,
>
> --
> Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
> ume at mahoroba.org  ume@{,jp.}FreeBSD.org
> http://www.imasy.org/~ume/
> ----
> Cyrus Home Page: http://cyrusimap.web.cmu.edu/
> Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
> List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
>