> Hi,
>
>>>>>> On Sun, 28 Oct 2007 19:42:02 +0100
>>>>>> Tomas Janousek <tjanouse@xxxxxxxxxx> 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@xxxxxxxxxxxx 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
>
----
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
