svn commit: r194493 - head/usr.bin/catman

Sat Jun 20 18:37:19 UTC 2009

On Sat, Jun 20, 2009 at 01:57:07PM +1000, Bruce Evans wrote:
> On Fri, 19 Jun 2009, Brooks Davis wrote:
> 
>> Log:
>>  When checking if we can write to a file, use access() instead of a
>>  manual permission check based on stat output.  Also, get rid of the
>>  executability check since it is not used.
> 
> This seems to add some security holes.  From "man assert | col -bx":
>
> %%%
> SECURITY CONSIDERATIONS
>      The access() system call is a potential security hole due to race condi-
>      tions and should never be used.  Set-user-ID and set-group-ID applica-
>      tions should restore the effective user or group ID, and perform actions
>      directly rather than use access() to simulate access checks for the real
>      user or group ID.	The eaccess() system call likewise may be subject to
>      races if used inappropriately.
> %%%

The code I replaced was effectivly a hand rolled version of access() based
on examining stat(1) results.  I think both are equivalently wrong from
a security perspective.

> catman isn't setuid in FreeBSD, so this doesn't exactly apply.  I think it
> does manual permission checking so as to support it being setuid on other
> systems.  It seems wrong to remove this support, provided it is actually
> correct.
> 
>> Modified: head/usr.bin/catman/catman.c
>> ==============================================================================
>> --- head/usr.bin/catman/catman.c	Fri Jun 19 15:31:40 2009	(r194492)
>> +++ head/usr.bin/catman/catman.c	Fri Jun 19 15:52:35 2009	(r194493)
>> @@ -759,14 +742,6 @@ main(int argc, char **argv)
>> {
>> 	int opt;
>> 
>> -	if ((uid = getuid()) == 0) {
>> -		fprintf(stderr, "don't run %s as root, use:\n   echo", argv[0]);
>> -		for (optind = 0; optind < argc; optind++) {
>> -			fprintf(stderr, " %s", argv[optind]);
>> -		}
>> -		fprintf(stderr, " | nice -5 su -m man\n");
>> -		exit(1);
>> -	}
>> 	while ((opt = getopt(argc, argv, "vnfLrh")) != -1) {
>> 		switch (opt) {
>> 		case 'f':
> 
> Surely it is wrong to remove the enforcement of not running as root?

Yes that was an error, I've restored that check.  Thanks for the catch.

-- Brooks

> FreeBSD seems to have removed all setuidness for saving of cat pages,
> resulting in saving of cat pages not actually working unless they
> are created by user man, either by user man viewing man pages or
> by su'ing to man to run catman as used to be commanded above, and
> then not subsequently clobbered by user root, either by user root
> viewing modified man pages or by running catman as root as used to
> be disallowed above.  I see the following brokenness starting with
> an empty /usr/share/man/cat1:
> - "man cp" as user bde doesn't save the cat page
> - "man cp" as user man saves the cat page, with correct ownwership "man"
>   After removing the cat page:
> - "man cp" as user root saves the cat page, with incorrect ownwership "root"
>   After changing the man page:
> - "man cp" as user bde displays the new man page but cannot save it
> - "man cp" as either user man or (of course) user root displays the new
>   man page and saves it.  Not too bad -- man(1) can replace the cat page
>   owned by root because user man owns the directory.  The mechanism is
>   that man(1) does a blind rename(2) of a temporary cat file.
> 
> man(1) was last setuid in 2002.  The log message for making it non-setuid
> explains why catman needs to be run to partially recover lost functionality:
> 
> %%%
> RCS file: /home/ncvs/src/gnu/usr.bin/man/man/Makefile,v
> Working file: Makefile
> head: 1.33
> ...
> ----------------------------
> revision 1.33
> date: 2002/01/15 14:11:05;  author: ru;  state: Exp;  lines: +1 -4
> branches:  1.33.30;  1.33.32;  1.33.34;
> Do not install man(1) setuid ``man''.
> 
> The catpaging and setuidness features of man(1) combined make
> it vulnerable to a number of security attacks.  Specifically,
> it was possible to overwrite system catpages with arbitrarily
> contents by either setting up a symlink to a directory holding
> system catpages, or by writing custom -mdoc or -man groff(1)
> macro packages and setting up GROFF_TMAC_PATH in environment
> to point to them.  (See PR below for details).
> 
> This means man(1) can no longer create system catpages on a
> regular user's behalf.  (It is still able to if the user has
> write permissions to the directory holding catpages, e.g.,
> user's own manpages, or if the running user is ``root''.)
> 
> To create and install catpages during ``make world'', please
> set MANBUILDCAT=YES in /etc/make.conf.  To rebuild catpages
> on a weekly basis, please set weekly_catman_enable="YES" in
> /etc/periodic.conf.
> 
> PR:		bin/32791
> ----------------------------
> %%%
> 
> I've never seen a machine that actually runs catman.  I remove the
> cat directories on all my machines to prevent any saving of cat
> pages.  All FreeBSD cluster machines that I checked (just 3) have
> 0, 1 and 2 saved cat pages.  This shows shows that catman isn't run
> on them and that root rarely looks at man pages on them.
> 
> Since catman must be run to create cat pages other than ones with
> incorrect ownership obtained by root looking at man pages, it might
> as always run as root.  User man and cat directories owned by user
> man would become completely useless instead of just useless.
> 
> Bruce
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20090620/009a887a/attachment.pgp