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

[Bruce Evans]

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.
%%%

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?

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