bug#8391: chmod setuid & setguid bits

[Eric Blake]

On 02/24/2012 05:53 AM, Ondrej Vasik wrote:

>> > 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
> 
> 
> chmod-octal.patch
> 
> 
>>From 4c31d59205b6558e0b217120649096890f00c679 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <address@hidden>
> 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.

Thanks for reviving this.

> +++ 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.
>  
> address@hidden 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.

5 or more digits.

> +For 4 digit octal mode ignores the leading zero digit, as this is condidered

s/sondidered/considered/

> +not explicit enough and incompatible with other implementations.

I'm not sure I like that wording.  How about:

@command{chmod} will not clear set-user-ID or set-group-ID bits of
@var{mode} of a directory when mode is specified as an octal number,
unless the mode had at least 5 digits (which implies a leading double
zero).  Preserving the special bits with four or fewer octal digits is
compatible with other implementations, to prevent opening an accidental
security hole on such a directory.

> @@ -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 */

/* Clear special bits on dirs only if 5 or more octal digits */

> +        keepdirbits = ((strlen(mode) != 5) || ('0' != *mode));

Spurious parenthesis.  I would write this as:

keepdirbits = 4 < strlen(mode);

After all, anyone passing 000755 still deserves to clear the special
bits, and anyone calling 11111 will get an error because 010000 is not a
valid mode bit, so a length check is sufficient.


> +++ 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

Also check for 000755.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature