Subject: bug#7320: [PATCH] 'id' prints incorrectly groups for the session

From: Pádraig Brady <[email-masked]>
Date: Thu, 26 Jun 2014 02:53:08 +0100

On 06/25/2014 01:17 PM, Petr Stodůlka wrote:
> Hi,
> 
> command 'id' prints wrong groups for the session. This is similar to reported bug #7320 [0],
> which was patched earlier for 'groups' and 'id -G', however just 'id' still prints wrong groups.
> I propose this patch based on previous solution:
> ----------------------------------------------------------------------
> diff --git a/src/id.c b/src/id.c
> index 3348f80..6cfe884 100644
> --- a/src/id.c
> +++ b/src/id.c
> @@ -399,8 +399,12 @@ print_full_info (const char *username)
>      gid_t *groups;
>      int i;
> 
> -    int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : -1),
> -                               &groups);
> +    int n_groups;
> +    if(username)
> +      n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : -1),
> +                             &groups);
> +    else
> +      n_groups = xgetgroups (username, egid, &groups);
>      if (n_groups < 0)
>        {
>          if (username)
> --------------------------------------------------------------------
> 
> [0] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7320

Logic looks correct.
The attached refactors slightly, and adds a test and NEWS.
I'll apply upon your ack.

thanks!
Pádraig.
>From 2fd678d1201dbb1c877aba139190b0ac1025964f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Stod=C5=AFlka?= <[email-masked]>
Date: Wed, 25 Jun 2014 18:26:23 +0100
Subject: [PATCH] id: output the effective group for the process

* src/id.c (print_full_info): When no user is specified,
output the effective group for the _process_, rather than
the default group from the system database, which may be different.
* tests/id/setgid.sh: Add a case for `id` as well as `id -G`.
* NEWS: Mention the bug fix.
Fixes http://bugs.gnu.org/7320
Reported at http://bugzilla.redhat.com/1016163
---
 NEWS               |    6 ++++++
 src/id.c           |   19 ++++++++++---------
 tests/id/setgid.sh |    9 +++++++--
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 6532785..e5ea77c 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   now copies all input to stdout.  Previously nothing was output in this case.
   [bug introduced with the --lines=-N feature in coreutils-5.0.1]
 
+  id, when invoked with no user name argument, now prints the correct group ID.
+  Previously, in the default output format, it would print the default group ID
+  in the password database, which may be neither real nor effective.  For e.g.,
+  when run set-GID, or when the database changes outside the current session.
+  [bug introduced in coreutils-8.1]
+
   ln -sf now replaces symbolic links whose targets can't exist.  Previously
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]
diff --git a/src/id.c b/src/id.c
index 3348f80..f46bb41 100644
--- a/src/id.c
+++ b/src/id.c
@@ -399,19 +399,20 @@ print_full_info (const char *username)
     gid_t *groups;
     int i;
 
-    int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : -1),
-                               &groups);
+    gid_t primary_group;
+    if (username)
+      primary_group = pwd ? pwd->pw_gid : -1;
+    else
+      primary_group = egid;
+
+    int n_groups = xgetgroups (username, primary_group, &groups);
     if (n_groups < 0)
       {
         if (username)
-          {
-            error (0, errno, _("failed to get groups for user %s"),
-                   quote (username));
-          }
+          error (0, errno, _("failed to get groups for user %s"),
+                 quote (username));
         else
-          {
-            error (0, errno, _("failed to get groups for the current process"));
-          }
+          error (0, errno, _("failed to get groups for the current process"));
         ok = false;
         return;
       }
diff --git a/tests/id/setgid.sh b/tests/id/setgid.sh
index aa43ea3..a81b42c 100755
--- a/tests/id/setgid.sh
+++ b/tests/id/setgid.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# Verify that id -G prints the right group when run set-GID.
+# Verify that id [-G] prints the right group when run set-GID.
 
 # Copyright (C) 2012-2014 Free Software Foundation, Inc.
 
@@ -27,9 +27,14 @@ gp1=$(expr $g + 1)
 
 echo $gp1 > exp || framework_failure_
 
+# With coreutils-8.16 and earlier, id -G would print both: $gp1 $g
 chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / 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
+
+# With coreutils-8.22 and earlier, id would erroneously print groups=$g
+chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / env PATH="$PATH" \
+  id > out || fail=1
+grep -F "groups=$gp1" out || fail=1
 
 Exit $fail
-- 
1.7.7.6