id,groups: with no user name, print only real and/or effective IDs,
authorJim Meyering <meyering@redhat.com>
Fri, 27 Apr 2012 11:28:32 +0000 (13:28 +0200)
committerJim Meyering <meyering@redhat.com>
Fri, 27 Apr 2012 17:54:03 +0000 (19:54 +0200)
... 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

Similarly, running those programs set-GID could make them
print one ID too many.

* 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.
* tests/misc/id-setgid: New file.
* tests/Makefile.am (TESTS): Add it.
(root_tests): It's a root-only test, so add it here, too.
Originally reported by Brynnen Owen as http://bugs.gnu.org/7320.
Raised again by Marc Mengel in http://bugzilla.redhat.com/816708.

NEWS
THANKS.in
src/group-list.c
tests/Makefile.am
tests/misc/id-setgid [new file with mode: 0755]

diff --git a/NEWS b/NEWS
index ef4e508..b7bfb8c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,15 @@ 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, when run
+  set-GID, or 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.
index d23f7b3..a7403fd 100644 (file)
--- a/THANKS.in
+++ b/THANKS.in
@@ -98,6 +98,7 @@ Brian Silverman                     bsilverman@conceptxdesign.com
 Brian Youmans                       3diff@gnu.org
 Britton Leo Kerin                   fsblk@aurora.uaf.edu
 Bruce Robertson                     brucer@theodolite.dyndns.org
+Brynnen Owen                        owen@illinois.edu
 Carl Johnson                        carlj@cjlinux.home.org
 Carl Lowenstein                     cdl@mpl.UCSD.EDU
 Carl Roth                           roth@urs.us
@@ -355,6 +356,7 @@ Manfred Hollstein                   manfred@s-direktnet.de
 Марк Коренберг                      socketpair@gmail.com
 Marc Boucher                        marc@mbsi.ca
 Marc Haber                          mh+debian-bugs@zugschlus.de
+Marc Mengel                         mengel@fnal.gov
 Marc Lehman                         schmorp@schmorp.de
 Marc Olzheim                        marcolz@stack.nl
 Marco Franzen                       Marco.Franzen@Thyron.com
index cf49911..edbb342 100644 (file)
@@ -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)
index ce2366b..72717e3 100644 (file)
@@ -36,6 +36,7 @@ root_tests =                                  \
   ls/nameless-uid                              \
   misc/chcon                                   \
   misc/chroot-credentials                      \
+  misc/id-setgid                               \
   misc/selinux                                 \
   misc/truncate-owned-by-other                 \
   mkdir/writable-under-readonly                        \
@@ -198,6 +199,7 @@ TESTS =                                             \
   misc/head-pos                                        \
   misc/id-context                              \
   misc/id-groups                               \
+  misc/id-setgid                               \
   misc/md5sum                                  \
   misc/md5sum-bsd                              \
   misc/md5sum-newline                          \
diff --git a/tests/misc/id-setgid b/tests/misc/id-setgid
new file mode 100755 (executable)
index 0000000..12fab38
--- /dev/null
@@ -0,0 +1,34 @@
+#!/bin/sh
+# Verify that id -G prints the right group when run set-GID.
+
+# Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ id
+require_root_
+
+g=$(id -u $NON_ROOT_USERNAME) || framework_failure_
+
+# Construct a different group number.
+gp1=$(expr $g + 1)
+
+echo $gp1 > exp || framework_failure_
+
+setuidgid -g $gp1 $NON_ROOT_USERNAME env PATH="$PATH" id -G > out || fail=1
+compare exp out || fail=1
+# With coreutils-8.16 and earlier, id -G would print both: $gp1 $g
+
+Exit $fail