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
>
pgpclys15LDoe.pgp
Description: PGP signature
