[newlib-cygwin/main] Cygwin: newgrp: only allow group from supplementary group list

[newlib-cygwin/main] Cygwin: newgrp: only allow group from supplementary group list

[Corinna Vinschen]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=0e3bb302dec35e79dc778c3bcd213e063335b67c

commit 0e3bb302dec35e79dc778c3bcd213e063335b67c
Author:     Corinna Vinschen <corinna@vinschen.de>
AuthorDate: Sat Feb 24 12:59:28 2024 +0100
Commit:     Corinna Vinschen <corinna@vinschen.de>
CommitDate: Sat Feb 24 13:08:34 2024 +0100

    Cygwin: newgrp: only allow group from supplementary group list
    
    Windows only allows to set the primary group to a group already
    present in the TOKEN_GROUP list.  Cygwin OTOH fakes success at
    setgid() time, to allow a subsequent call to setuid() to do
    the actual account switching.  To have a sane behaviour in the
    command line tool, check group membership and disallow to switch
    to groups other than those already present in the user token.
    
    Fixes: 8bd56ec873453 ("Cygwin: newgrp: first full version")
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/doc/utils.xml  |  8 +++-----
 winsup/utils/newgrp.c | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/winsup/doc/utils.xml b/winsup/doc/utils.xml
index 1d3fe909b0d2..b93671b16dba 100644
--- a/winsup/doc/utils.xml
+++ b/winsup/doc/utils.xml
@@ -2116,11 +2116,9 @@ D: on /d type fat (binary,user,noumount)
 	has been given as argument, a command and its arguments can be
 	specified on the command line.</para>
 
-      <para>Please note that setting the primary group to any arbitrary group
-        is no privileged operation on Windows.  However, even if this group is
-	not in your current user token, or if the group is in your user token
-	but marked as <literal>deny-only</literal>, no additional permissions
-	can be obtained by setting this group as primary group.</para>
+      <para>The new primary group must be either the old primary group, or
+        it must be part of the supplementary group list.  Setting the primary
+	group to an arbitrary group is not allowed in Windows.</para>
     </refsect1>
 
     <refsect1 id="newgrp-seealso">
diff --git a/winsup/utils/newgrp.c b/winsup/utils/newgrp.c
index e312a3c51d38..414e8cdf8edc 100644
--- a/winsup/utils/newgrp.c
+++ b/winsup/utils/newgrp.c
@@ -136,6 +136,7 @@ main (int argc, const char **argv)
   char **child_env;
   bool new_child_env = false;
   gid_t gid;
+  int ngrps;
 
   setlocale (LC_ALL, "");
 
@@ -176,6 +177,29 @@ main (int argc, const char **argv)
       ++argv;
     }
 
+  /* Windows does not allow to set the primary group to another group if
+     it's not already part of the supplementary group list.  However, our
+     setgid() allows this, otherwise OpenSSH and other account-switching
+     processes wouldn't work, given we only actually switch the user
+     context at setuid() time.  Therefore we test this here and don't
+     allow other groups. */
+  ngrps = getgroups (0, NULL);
+  if (ngrps > 0)
+    {
+      gid_t *glist = (gid_t *) alloca (ngrps * sizeof (gid_t));
+
+      ngrps = getgroups (ngrps, glist);
+	while (--ngrps >= 0)
+	  if (gid == glist[ngrps])
+	    break;
+      if (ngrps < 0)
+	{
+	  fprintf (stderr, "%s: can't switch primary group to '%s'\n",
+		   program_invocation_short_name, gr->gr_name);
+	  return 2;
+	}
+    }
+
   /* Set primary group */
   if (setgid (gid) != 0)
     {