From owner-svn-src-head@FreeBSD.ORG Sat Jun 20 18:37:18 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D08C91065757; Sat, 20 Jun 2009 18:37:18 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net (cl-162.ewr-01.us.sixxs.net [IPv6:2001:4830:1200:a1::2]) by mx1.freebsd.org (Postfix) with ESMTP id 498CE8FC12; Sat, 20 Jun 2009 18:37:18 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net (localhost [127.0.0.1]) by lor.one-eyed-alien.net (8.14.3/8.14.3) with ESMTP id n5KIbTPX079999; Sat, 20 Jun 2009 13:37:29 -0500 (CDT) (envelope-from brooks@lor.one-eyed-alien.net) Received: (from brooks@localhost) by lor.one-eyed-alien.net (8.14.3/8.14.3/Submit) id n5KIbTpP079998; Sat, 20 Jun 2009 13:37:29 -0500 (CDT) (envelope-from brooks) Date: Sat, 20 Jun 2009 13:37:29 -0500 From: Brooks Davis To: Bruce Evans Message-ID: <20090620183728.GA72393@lor.one-eyed-alien.net> References: <200906191552.n5JFqZcG047705@svn.freebsd.org> <20090620130238.N29302@delplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cWoXeonUoKmBZSoM" Content-Disposition: inline In-Reply-To: <20090620130238.N29302@delplex.bde.org> User-Agent: Mutt/1.5.17 (2007-11-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-3.0 (lor.one-eyed-alien.net [127.0.0.1]); Sat, 20 Jun 2009 13:37:29 -0500 (CDT) Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Brooks Davis , src-committers@FreeBSD.org Subject: Re: svn commit: r194493 - head/usr.bin/catman X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Jun 2009 18:37:19 -0000 --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--