bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#10021: [PATCH id] Add error-checking on GNU


From: Paul Eggert
Subject: Re: bug#10021: [PATCH id] Add error-checking on GNU
Date: Mon, 14 Nov 2011 10:54:32 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0

On 11/14/11 00:52, Jim Meyering wrote:
> I tested this on a gnu/linux system and found that I could indeed create a
> user with a UID of 2^32-1

If we really want to support that, there would be
a lot of other coreutils code that would need fixing,
no?  For example, the command

  chown 4294967295 /tmp/foo

always fails, but presumably on your host which has a
user (call it ubig) with UID 4294967295, the command

  chown ubig /tmp/foo

is a no-op?  Ouch.

I can see where this would have some serious security
implications.  Instead of trying to cater to systems
like that, it's probably better just to say "Don't Do That",
i.e., GNU/Linux systems that have a user with ID == (uid_t) -1
are improperly set up and coreutils does not especially
cater to such systems.

That being said, it's OK if 'id' does support such systems,
(if only to diagnose their brokenness :-).  But looking at
the current code, id is making the unportable assumption that
uid_t is at least as wide as int.  And the way that id works
around the GNU porting problem isn't quite right, as there
are systems other than GNU that have the problem.

How about this patch?  It's simpler and should fix the
issues mentioned in the previous paragraph.

id: handle (uid_t) -1 more portably
* src/id.c (GETID_MAY_FAIL): Remove.
(main): Check for negative return values, not for -1.
The old code was incorrect if uid_t was narrower than int,
regardless of whether we were on a GNU or a POSIX platform.
The new code is simpler and doesn't need GETID_MAY_FAIL.
(print_full_info): Remove unnecessary cast to -1.
diff --git a/src/id.c b/src/id.c
index 047e40b..7bd8ebd 100644
--- a/src/id.c
+++ b/src/id.c
@@ -38,13 +38,6 @@
   proper_name ("Arnold Robbins"), \
   proper_name ("David MacKenzie")

-/* Whether the functions getuid, geteuid, getgid and getegid may fail.  */
-#ifdef __GNU__
-# define GETID_MAY_FAIL 1
-#else
-# define GETID_MAY_FAIL 0
-#endif
-
 /* If nonzero, output only the SELinux context. -Z */
 static int just_context = 0;

@@ -208,22 +201,35 @@ main (int argc, char **argv)
     }
   else
     {
+      /* On GNU hosts, getuid etc. can fail and return -1.  On POSIX
+         hosts, such failures are not allowed and (uid_t) -1 may be a
+         valid UID if uid_t is unsigned.  To handle both cases
+         correctly, consider getuid etc. to fail if it returns a
+         negative value.
+
+         POSIX hosts should not give users a UID equal to (uid_t) -1
+         even if uid_t is unsigned, as system calls like chown would
+         behave unexpectedly with such a UID, and in general coreutils
+         therefore does not support such a UID.  However, 'id' makes a
+         special attempt to handle it, if only to help people diagnose
+         the problem.  */
+
       euid = geteuid ();
-      if (GETID_MAY_FAIL && euid == -1 && !use_real
+      if (euid < 0 && !use_real
           && !just_group && !just_group_list && !just_context)
         error (EXIT_FAILURE, errno, _("cannot get effective UID"));

       ruid = getuid ();
-      if (GETID_MAY_FAIL && ruid == -1 && use_real
+      if (ruid < 0 && use_real
           && !just_group && !just_group_list && !just_context)
         error (EXIT_FAILURE, errno, _("cannot get real UID"));

       egid = getegid ();
-      if (GETID_MAY_FAIL && egid == -1 && !use_real && !just_user)
+      if (egid < 0 && !use_real && !just_user)
         error (EXIT_FAILURE, errno, _("cannot get effective GID"));

       rgid = getgid ();
-      if (GETID_MAY_FAIL && rgid == -1 && use_real && !just_user)
+      if (rgid < 0 && use_real && !just_user)
         error (EXIT_FAILURE, errno, _("cannot get real GID"));
     }

@@ -316,7 +322,7 @@ print_full_info (const char *username)
     gid_t *groups;
     int i;

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]