Date: Sat, 20 Jun 2009 13:37:29 -0500 From: Brooks Davis <brooks@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Brooks Davis <brooks@FreeBSD.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r194493 - head/usr.bin/catman Message-ID: <20090620183728.GA72393@lor.one-eyed-alien.net> In-Reply-To: <20090620130238.N29302@delplex.bde.org> References: <200906191552.n5JFqZcG047705@svn.freebsd.org> <20090620130238.N29302@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--cWoXeonUoKmBZSoM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Jun 20, 2009 at 01:57:07PM +1000, Bruce Evans wrote:
> On Fri, 19 Jun 2009, Brooks Davis wrote:
>=20
>> 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.
>=20
> 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 co=
ndi-
> tions and should never be used. Set-user-ID and set-group-ID applic=
a-
> tions should restore the effective user or group ID, and perform act=
ions
> 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.
>=20
>> Modified: head/usr.bin/catman/catman.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- 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;
>>=20
>> - if ((uid =3D getuid()) =3D=3D 0) {
>> - fprintf(stderr, "don't run %s as root, use:\n echo", argv[0]);
>> - for (optind =3D 0; optind < argc; optind++) {
>> - fprintf(stderr, " %s", argv[optind]);
>> - }
>> - fprintf(stderr, " | nice -5 su -m man\n");
>> - exit(1);
>> - }
>> while ((opt =3D getopt(argc, argv, "vnfLrh")) !=3D -1) {
>> switch (opt) {
>> case 'f':
>=20
> 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 "ro=
ot"
> 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.
>=20
> 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 functionali=
ty:
>=20
> %%%
> 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''.
>=20
> 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).
>=20
> 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''.)
>=20
> To create and install catpages during ``make world'', please
> set MANBUILDCAT=3DYES in /etc/make.conf. To rebuild catpages
> on a weekly basis, please set weekly_catman_enable=3D"YES" in
> /etc/periodic.conf.
>=20
> PR: bin/32791
> ----------------------------
> %%%
>=20
> 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.
>=20
> 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.
>=20
> Bruce
>=20
--cWoXeonUoKmBZSoM
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)
iD8DBQFKPSxoXY6L6fI4GtQRAjzHAJ9phoUaYrUu8IRDs1TKjwxAVOCcqQCfaCJ6
groE7ez5e0lgPUVY+vyeNsc=
=8vKe
-----END PGP SIGNATURE-----
--cWoXeonUoKmBZSoM--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090620183728.GA72393>