cvs admin incorrectly rejects users whose primary group is cvsadm in

[Okken, Thomas]

Dear cvs-bug,

I ran into the following bug with CVS 1.10 sunning on Solaris 2.6 (Sparc): when trying to use the "cvs admin" command to change the RCS properties of a file, I got the error "usage is restricted to members of the group cvsadmin" - despite the fact that the user I was running as *is* in fact in the cvsadmin group.

The relevant lines from /etc/group and /etc/passwd look like this:

/etc/passwd: cvsadmin:x:10001:105::/export/home/cvs:/bin/ksh
/etc/group: cvsadmin::105:

This is the section from src/admin.c that checks for membership in the cvsadmin group:

#ifdef CVS_ADMIN_GROUP
    grp = getgrnam(CVS_ADMIN_GROUP);
     /* skip usage right check if group CVS_ADMIN_GROUP does not exist */
    if (grp != NULL)
    {
        char *me = getcaller();
        char **grnam = grp->gr_mem;
        /* The use of `cvs admin -k' is unrestricted.  However, any
           other option is restricted.  */
        int denied = ! only_k_option;

        while (*grnam)
        {
            if (strcmp(*grnam, me) == 0)
            {
                denied = 0;
                break;
            }
            grnam++;
        }

        if (denied)
            error (1, 0, "usage is restricted to members of the group %s",
                   CVS_ADMIN_GROUP);
    }
#endif

I have tried compiling and executing the getgrnam() call in a standalone test program, and it turns out that it does not return all users that are members of the given group; rather, it only returns the users that are listed as member of the given group *in /etc/group*. In other words, users (such as cvsadmin in my example) whose primary group is cvsadmin, but who are not listed as cvsadmin members in /etc/group, will *not* show up in the output from getgrnam(), even though the Unix 'groups' command confirms that they are, in fact, members of that group.

As a workaround, we got our sysadmin to add the cvsadmin username to the cvsadmin group in /etc/group, which solves the problem. It would be better to fix CVS, though (unless the behavior of getgrnam() in Solaris 2.6 is buggy; I don't know what POSIX has to say about this -- but even then, fixing CVS would be nice, you just wouldn't call it a fix, but a bug workaround).

Here's my suggestion for how to change the above piece of code:

#ifdef CVS_ADMIN_GROUP
    grp = getgrnam(CVS_ADMIN_GROUP);
     /* skip usage right check if group CVS_ADMIN_GROUP does not exist */
    if (grp != NULL)
    {
        char *me = getcaller();
        struct passwd *my_pwd = getpwnam(me);
        if (my_pwd->pw_gid == grp->gr_gid)
            goto not_denied;
        char **grnam = grp->gr_mem;
        /* The use of `cvs admin -k' is unrestricted.  However, any
           other option is restricted.  */
        int denied = ! only_k_option;

        while (*grnam)
        {
            if (strcmp(*grnam, me) == 0)
            {
                denied = 0;
                break;
            }
            grnam++;
        }

        if (denied)
            error (1, 0, "usage is restricted to members of the group %s",
                   CVS_ADMIN_GROUP);
    }
    not_denied:
#endif

In addition to the three lines added here, at the head of the file, #include <pwd.h> is also needed.
This would fix the problem, at least for our environment. (Note: we're actually using a binary distribution of 1.10, not 1.11, but this code was not changed between those versions.) This code may not be necessary on other Unixes, depending on the return value from getgrnam(), but it would not do any harm either.

Hope this helps; if you require more information, do not hesitate to contact me.
Thanks!

 - Thomas Okken