Subject: Re: bug#8391: chmod setuid & setguid bits

From: Ondrej Vasik <[email-masked]>
Date: Fri, 24 Feb 2012 13:53:17 +0100

On Mon, 2011-04-04 at 11:04 +0200, Ondrej Vasik wrote:
> Hi,
> On Thu, 2011-03-31 at 14:15 -0600, Eric Blake wrote:
> > On 03/31/2011 01:58 PM, Christian wrote:
> > > Hi Paul,
> > > 
> > > Am 31.03.2011 20:54, schrieb Paul Eggert:
> > >> On 03/31/2011 11:25 AM, Christian wrote:
> > >>> and using "0755" is explicit enough, isn't it ?
> > >> Unfortunately it's not that simple, as having 0755 mean
> > >> something different from 755 would violate the principle
> > >> of least surprise.  Please see the thread starting at
> > >> <http://lists.gnu.org/archive/html/bug-coreutils/2006-07/msg00124.html>.
> > > I read it and I came to the conclusion
> > > 755 should preserve s-bit: OK
> > > 2755 sould set sbit. OK
> > > 0755 should remove sbit, cause it is explicit wanted.
> > > and not doing so is a "lemming behaviour".
> > 
> > No, 0755 is not explicit - it is ambiguous with people that are
> > explicitly using printf %#3o to output a 3-digit octal string with
> > leading 0 - I don't think we can change this.
> 
> I just want to mention recent RH bugzilla report about the same issue -
> https://bugzilla.redhat.com/show_bug.cgi?id=691466 ... one of the ideas
> in this bugzilla is that because of the ambiguity of the 0755 (and
> similars) it would be very good to actually inform user in the case that
> the special bit was not cleared (so only in the case that
> SUID/SGID/sticky bits are set). Maybe only in verbose mode, but I think
> it will be improvement for the current situation. What do you think?
> 
> > But my suggestion of 00755 _is_ explicit - after taking off the leading
> > 0 for specifying octal, you are _still_ left with four octal digits
> > which includes the sticky bit explicitly being set to 0.
> 
> 
> I really like the 00XXX suggestion - do you plan to implement that
> yourself? If you don't have time for writing it but this solution is
> generally acceptable compromise, I could try to prepare a patch for
> that.

Sorry for late patch...
Double zero five octal digits modes cleaning change, with test (and info
documentation clarification) is in attachment.

Greetings,
         Ondrej Vasik

>From 4c31d59205b6558e0b217120649096890f00c679 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email-masked]>
Date: Fri, 24 Feb 2012 13:34:35 +0100
Subject: [PATCH] chmod: Clear special bits for octal modes specified by 5 digits.
         * src/chmod.c : Use new keepdirbits boolean for clearing special
                         bits for directories for double leading zero octal
                         mode.
         * NEWS: Mention the change.
         * doc/coreutils.texi (chmod invocation): Document the change.
         * tests/chmod/setuid : Check the new behaviour by test.
         Suggested by Eric Blake.

---
 NEWS               |    3 +++
 doc/coreutils.texi |    6 ++++++
 src/chmod.c        |   16 ++++++++++++----
 tests/chmod/setgid |    9 +++++++++
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index e2e8fc5..e2e61cb 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   systems for which getfilecon-, ACL-check- and XATTR-check-induced syscalls
   fail with ENOTSUP or similar.
 
+  chmod now can clear the setuid and setgid bits on directories
+  for mode specified as 5 octal digits (e.g. 00755).
+
 
 * Noteworthy changes in release 8.15 (2012-01-06) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index cc300a8..8e13ba3 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10208,6 +10208,12 @@ may cause the set-user-ID and set-group-ID bits of @var{mode} or
 functionality of the underlying @code{chmod} system call.  When in
 doubt, check the underlying system behavior.
 
+@command{chmod} by default keeps the set-user-ID and set-group-ID bits
+of @var{mode} of a directory when the mode is specified as an octal digit,
+unless the mode length is 5 digits with leading double zero.
+For 4 digit octal mode ignores the leading zero digit, as this is condidered
+not explicit enough and incompatible with other implementations.
+
 If used, @var{mode} specifies the new file mode bits.
 For details, see the section on @ref{File permissions}.
 If you really want @var{mode} to have a leading @samp{-}, you should
diff --git a/src/chmod.c b/src/chmod.c
index a134e3f..7bbce51 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -68,6 +68,9 @@ static mode_t umask_value;
 /* If true, change the modes of directories recursively. */
 static bool recurse;
 
+/* if true, keep the special directory access bits */
+static bool keepdirbits = true;
+
 /* If true, force silence (suppress most of error messages). */
 static bool force_silent;
 
@@ -263,8 +266,9 @@ process_file (FTS *fts, FTSENT *ent)
   if (ok)
     {
       old_mode = file_stats->st_mode;
-      new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value,
-                              change, NULL);
+      new_mode = mode_adjust (old_mode,
+                              (S_ISDIR (old_mode) != 0) && keepdirbits,
+                              umask_value, change, NULL);
 
       if (! S_ISLNK (old_mode))
         {
@@ -299,7 +303,8 @@ process_file (FTS *fts, FTSENT *ent)
   if (chmod_succeeded && diagnose_surprises)
     {
       mode_t naively_expected_mode =
-        mode_adjust (old_mode, S_ISDIR (old_mode) != 0, 0, change, NULL);
+        mode_adjust (old_mode, (S_ISDIR (old_mode) != 0) && keepdirbits,
+                     0, change, NULL);
       if (new_mode & ~naively_expected_mode)
         {
           char new_perms[12];
@@ -513,8 +518,11 @@ main (int argc, char **argv)
     }
   else
     {
-      if (!mode)
+      if (!mode) {
         mode = argv[optind++];
+        /* Clean special bits on dirs for 5 digits octal with leading zero */
+        keepdirbits = ((strlen(mode) != 5) || ('0' != *mode));
+      }
     }
 
   if (optind >= argc)
diff --git a/tests/chmod/setgid b/tests/chmod/setgid
index eaa9351..95e164c 100755
--- a/tests/chmod/setgid
+++ b/tests/chmod/setgid
@@ -45,4 +45,13 @@ chmod 755 d
 
 case `ls -ld d` in drwxr-sr-x*);; *) fail=1;; esac
 
+# make sure that it doesn't clear the bits for 4 digit octal mode
+chmod 0755 d
+case `ls -ld d` in drwxr-sr-x*);; *) fail=1;; esac
+
+
+# make sure that it clears the bits for 5 digit octal mode with leading zero
+chmod 00755 d
+case `ls -ld d` in drwxr-xr-x*);; *) fail=1;; esac
+
 Exit $fail
-- 
1.7.1