GNU bug report logs - #7320
'group' command gives wrong/extra group

Previous Next

Full log

View this message in rfc822 format

From: Jim Meyering <jim <at> meyering.net>
To: "Marc W. Mengel" <mengel <at> fnal.gov>, owen <at> illinois.edu
Cc: 7320 <at> debbugs.gnu.org
Subject: bug#7320: id and groups may lie
Date: Fri, 27 Apr 2012 15:43:37 +0200

Jim Meyering wrote:
> Marc W. Mengel wrote:
>> This is still broken in RedHat in coreutils-8.4-13
>>
>> All of  "groups" and "id" and "id -G" report groups that you don't have
>> if you list a new/different primary group in /etc/passwd.
>>
>> This is just plain wrong.  "id" and "groups" should list the groups you
>> actually have, not what you would possibly have if you logged out and
>> back in again.
>
> Thank you for the report.
> It looks like there is indeed a bug.
>
> I demonstrated it with this:
...
> With all that, here's the patch I expect to commit:
>
> diff --git a/src/group-list.c b/src/group-list.c

Here's a complete patch.
Note the lack of a test case.
Even in a root-only test, and briefly, I don't want to change
the password database.

From 3bcb3ea46d685f499c7a02efb1cbbbf15f858325 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Fri, 27 Apr 2012 13:28:32 +0200
Subject: [PATCH] id,groups: with no user name, print only real and/or
 effective IDs,

... i.e., don't use the getpw* functions.
Before this change, running
groups or id with no user name argument would include a group
name or ID from /etc/passwd.  Thus, under unusual circumstances
(default group is changed, but has not taken effect for a given
session), those programs could print a name or ID that is neither
real nor effective.

To demonstrate, run this:

    echo 'for i in 1 2; do id -G; sleep 1.5; done' \
      |su -s /bin/sh ftp - &
    sleep 1; perl -pi -e 's/^(ftp:x:\d+):(\d+)/$1:9876/' /etc/passwd

Those id -G commands printed the following:

    50
    50 9876

With this change, they print this:

    50
    50

* src/group-list.c (print_group_list): When username is NULL, pass
egid, not getpwuid(ruid)->pw_gid), to xgetgroups, per the API
requirements of xgetgroups callee, mgetgroups.
When not using the password database, don't call getpwuid.
* NEWS (Bug fixes): Mention it.
Originally reported by Brynnen Owen as http://bugs.gnu.org/7320.
Raised again by Marc Mengel in http://bugzilla.redhat.com/816708.
---
 NEWS             |    8 ++++++++
 THANKS.in        |    2 ++
 src/group-list.c |   14 ++++++++------
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index ef4e508..c50336b 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@ GNU coreutils NEWS                                    -*- outline -*-

 * Noteworthy changes in release ?.? (????-??-??) [?]

+** Bug fixes
+
+  id and groups, when invoked with no user name argument, would print
+  the default group ID listed in the password database, and sometimes
+  that ID would be neither real nor effective.  For example, in a session
+  for which the default group has just been changed, the new group ID
+  would be listed, even though it is not yet effective.
+
 ** New features

   fmt now accepts the --goal=WIDTH (-g) option.
diff --git a/THANKS.in b/THANKS.in
index d23f7b3..a7403fd 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -98,6 +98,7 @@ Brian Silverman                     bsilverman <at> conceptxdesign.com
 Brian Youmans                       3diff <at> gnu.org
 Britton Leo Kerin                   fsblk <at> aurora.uaf.edu
 Bruce Robertson                     brucer <at> theodolite.dyndns.org
+Brynnen Owen                        owen <at> illinois.edu
 Carl Johnson                        carlj <at> cjlinux.home.org
 Carl Lowenstein                     cdl <at> mpl.UCSD.EDU
 Carl Roth                           roth <at> urs.us
@@ -355,6 +356,7 @@ Manfred Hollstein                   manfred <at> s-direktnet.de
 Марк Коренберг                      socketpair <at> gmail.com
 Marc Boucher                        marc <at> mbsi.ca
 Marc Haber                          mh+debian-bugs <at> zugschlus.de
+Marc Mengel                         mengel <at> fnal.gov
 Marc Lehman                         schmorp <at> schmorp.de
 Marc Olzheim                        marcolz <at> stack.nl
 Marco Franzen                       Marco.Franzen <at> Thyron.com
diff --git a/src/group-list.c b/src/group-list.c
index cf49911..edbb342 100644
--- a/src/group-list.c
+++ b/src/group-list.c
@@ -38,11 +38,14 @@ print_group_list (const char *username,
                   bool use_names)
 {
   bool ok = true;
-  struct passwd *pwd;
+  struct passwd *pwd = NULL;

-  pwd = getpwuid (ruid);
-  if (pwd == NULL)
-    ok = false;
+  if (username)
+    {
+      pwd = getpwuid (ruid);
+      if (pwd == NULL)
+        ok = false;
+    }

   if (!print_group (rgid, use_names))
     ok = false;
@@ -58,8 +61,7 @@ print_group_list (const char *username,
     gid_t *groups;
     int i;

-    int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1),
-                               &groups);
+    int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : egid), &groups);
     if (n_groups < 0)
       {
         if (username)
--
1.7.10.336.gc5e31

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.