Date: Fri, 5 Jan 2001 18:36:34 -0500 (EST) From: Robert Watson <rwatson@FreeBSD.org> To: Dag-Erling Smorgrav <des@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.bin/finger finger.1 lprint.c pathnames.h Message-ID: <Pine.NEB.3.96L.1010105182050.6061A-100000@fledge.watson.org> In-Reply-To: <200012291139.eBTBdQ251788@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Frankly, I was pretty disappointed to see this commit go in. The last time a change was made to the finger command, a substantial remotely-exploitable security hole was introduced, resulting in security advisories, easy exploitation, etc. While I have great confidence in your ability to discover and fix such problems in committed code, this type of (gratuitous) change worries me. In the future, it would lower my blood pressure (and lower risk of security holes slipping through the cracks) if you could run changes in sensitive (i.e., network-exposed) software by the freebsd-audit mailing list. I realize this particular change is not as risky as the last change, but it's good practice, and there are a number of people on the list quite happy to do audits of code on request. In general, (suggested policy statement here), the following types of code should be audited for potential holes before committing (and all of which have involved serious security holes recently): - Network daemons, or utilities frequently invoked by network daemons (inetd, fingerd, finger, ...) - Setuid/Setgid binaries (passwd, top, ...) - Authentication and authorization framework (login, PAM modules, Kerberos) - Libraries used by the previous three classes of code (in particular, libraries sensitive to string/environmental variable contents) - Kernel code that manages or responds to privilege or credentials (in particular, setuid() and related gid calls, module loading code, and so on). - Kernel code that does not use the new safe string management routines (i.e., uses strcpy and so on, and in particular, uses sprintf). The -audit list clearly has finite resources for looking for problems, but giving a heads up and URL for patches (or preferably attaching them) gives reviewers the opportunity to take a look and see if there are any immediate concerns worth putting off the commit for. The lessons from the last finger hole included: committing bad code to -CURRENT easily results in the code making it into a release, especially if you MFC in a hurry before the release, that it's hard for auditors to identify risky commits catch holes after they're committed due to the volume of commits unless they're given a heads up, and that even really simple code errors can have dramatic consequences when the code is exposed to risky input (such as finger, which is invoked anonymously over the network in most configurations). Needless to say, we'd welcome more people to sit on the list. In a sense, DES is a poor target for my deciding to send out a blast relating to code auditing, as he's responsible for our recent safe string library inclusion in kernel, and has a track record of good code -- on the other hand, given experiences, I think that this message needs to be sent for the broader developer community :-). Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services On Fri, 29 Dec 2000, Dag-Erling Smorgrav wrote: > des 2000/12/29 03:39:25 PST > > Modified files: > usr.bin/finger finger.1 lprint.c pathnames.h > Log: > Add support for a .publickey file. > > Submitted by: Svein Skogen <tds@nsn.no> > Reviewed by: brian, ru > > Revision Changes Path > 1.18 +7 -5 src/usr.bin/finger/finger.1 > 1.13 +3 -1 src/usr.bin/finger/lprint.c > 1.3 +2 -1 src/usr.bin/finger/pathnames.h > > > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1010105182050.6061A-100000>