Re: [bug #28113] chown silently fails to set uid/gid of ([ug]id_t) -1

[Jim Meyering]

Matt McCutchen wrote:
>   <http://savannah.gnu.org/bugs/?28113>
>
>                  Summary: chown silently fails to set uid/gid of ([ug]id_t) -1
>
> Details:
>
> chown(2) and related system calls treat ([ug]id_t) -1 as a special value
> meaning "don't change this field".  If chown(1) is called with a uid or gid of
> ([ug]id_t) -1 (typically 4294967295), it naively passes the value on to the
> system call and, as a result, silently fails to do what the user asked.  It
> should issue an error message in this case.  If the user had actually wanted
> to not change a particular field, he/she would have used the appropriate
> chown(1) syntax: "OWNER:" or ":GROUP".
>
> Steps to reproduce:
> $ touch test
> $ chown 4294967295:4294967295 test
> No error, but "test" still has the same ownership as before.
>
> Tested with Fedora coreutils-7.2-4.fc11.x86_64 and with the latest coreutils
> from the repository.
>
> The problem came up at:
> https://sourceforge.net/mailarchive/forum.php?thread_name=1259202707.2009.382.camel%40mattlaptop2.local&forum_name=rsnapshot-discuss

Thanks for the bug report.
I've begun addressing this.
The first step is to make gnulib's userspec module do the right thing:
[then I'll add unit tests in gnulib and coreutils]

>From 2ead6c09f1bda238b1ccd923f9983b7744581c83 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 28 Nov 2009 07:33:16 +0100
Subject: [PATCH] userspec: disallow an ID that maps to (uid_t)-1 or (gid_t)-1

* lib/userspec.c (parse_with_separator): Do not accept a user ID
number of MAXUID when it evaluates to (uid_t) -1.
Likewise for group ID.  Reported by Matt McCutchen in
<http://savannah.gnu.org/bugs/?28113>
---
 ChangeLog      |    6 ++++++
 lib/userspec.c |    5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e274ef7..0f130f5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-11-28  Jim Meyering  <address@hidden>

+       userspec: disallow an ID that maps to (uid_t)-1 or (gid_t)-1
+       * lib/userspec.c (parse_with_separator): Do not accept a user ID
+       number of MAXUID when it evaluates to (uid_t) -1.
+       Likewise for group ID.  Reported by Matt McCutchen in
+       <http://savannah.gnu.org/bugs/?28113>
+
        userspec: reformat to use spaces, not TABs
        * lib/userspec.c: Expand TABs to spaces.
        Add Emacs' "indent-tabs-mode: nil" hint.
diff --git a/lib/userspec.c b/lib/userspec.c
index fa2f26f..0388cb1 100644
--- a/lib/userspec.c
+++ b/lib/userspec.c
@@ -169,7 +169,7 @@ parse_with_separator (char const *spec, char const 
*separator,
             {
               unsigned long int tmp;
               if (xstrtoul (u, NULL, 10, &tmp, "") == LONGINT_OK
-                  && tmp <= MAXUID)
+                  && tmp <= MAXUID && (uid_t) tmp != (uid_t) -1)
                 unum = tmp;
               else
                 error_msg = E_invalid_user;
@@ -200,7 +200,8 @@ parse_with_separator (char const *spec, char const 
*separator,
       if (grp == NULL)
         {
           unsigned long int tmp;
-          if (xstrtoul (g, NULL, 10, &tmp, "") == LONGINT_OK && tmp <= MAXGID)
+          if (xstrtoul (g, NULL, 10, &tmp, "") == LONGINT_OK
+              && tmp <= MAXGID && (gid_t) tmp != (gid_t) -1)
             gnum = tmp;
           else
             error_msg = E_invalid_group;
--
1.6.6.rc0.285.g73651