Date: Fri, 16 Feb 2001 14:46:48 -0500 (EST) From: Mike Heffner <mheffner@vt.edu> To: "Jacques A. Vidrine" <n@nectar.com> Cc: Nathan Ahlstrom <nrahlstr@winternet.com>, FreeBSD-audit <FreeBSD-audit@FreeBSD.ORG> Subject: Re: mail(1) cleanup patch Message-ID: <XFMail.20010216144648.mheffner@vt.edu> In-Reply-To: <20010216082420.A85640@hamlet.nectar.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format --_=XFMail.1.4.7.FreeBSD:20010216144647:96217=_ Content-Type: text/plain; charset=us-ascii On 16-Feb-2001 Jacques A. Vidrine wrote: | On Thu, Feb 15, 2001 at 11:44:19PM -0500, Mike Heffner wrote: |> http://filebox.vt.edu/users/mheffner/patches/mail.patch | | This looks good. There are always more nits :-) but mostly they were | there before you arrived. Nothing earth-shattering, I think, but hey, | ``while you are there'': | | Purely -audit fodder: | = Are you certain that in each case where you've used | snprintf/strlcpy, that truncation is harmless? Perhaps better | to check. Stuff like `cp += strlcpy(...)' is particularly | suspect. I added truncation checking to a few that I put in, I'll look over the others again. The particular case you mention, `cp += strlcpy(..', is safe though because the previous code checks the lengths. | = Paranoia about strcpy->strlcpy is good, but then there are some | calls that could be converted from strcat->strlcat. There are only two strcat()'s that concat a single character, and the statements before assures there'll be enough space for it. | Stuff I couldn't keep to myself (I tried): | = anyof() can be tossed, and strpbrk() used directly. Good idea, it looks like it's only called once anyways. | = There's some inconsistent usage of err vs errx. I've tried to use err() whenever errno would be set and errx() otherwise. However, I did just find a few cases where errno would be set and I had used errx(), and vice-versa. Fixed. | = NOSTR, NIL, NONE, NOVAR, NOGRP, NOGE are misspellings of NULL. I plan to commit these changes in a followup style patch. | = creat is the ancient way of spelling open(..., O_CREAT|O_TRUNC|O_WRONLY, | ...); I'll change this too. | | Thanks for the hard work! Thanks for taking the time to review. :) P.S. I'll update the patch either later tonight or tomorrow. -- Mike Heffner <mheffner@vt.edu> Blacksburg, VA <mikeh@FreeBSD.org> http://filebox.vt.edu/users/mheffner --_=XFMail.1.4.7.FreeBSD:20010216144647:96217=_ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.4 (FreeBSD) Comment: For info see http://www.gnupg.org iD8DBQE6jYOnFokZQs3sv5kRAhLgAJ9FyT4GpMuLfb4BbD0u3HFi0Dy/8QCfYAAP +Jdzm3WBTlG9N8YLiwWKej4= =Kjfv -----END PGP SIGNATURE----- --_=XFMail.1.4.7.FreeBSD:20010216144647:96217=_-- End of MIME message To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20010216144648.mheffner>