Date: Fri, 5 Oct 2001 09:07:27 +0300 From: Peter Pentchev <roam@ringlet.net> To: Mike Barcroft <mike@FreeBSD.ORG> Cc: "Todd C. Miller" <Todd.Miller@courtesan.com>, freebsd-net@FreeBSD.ORG, freebsd-audit@FreeBSD.ORG Subject: Re: [CFR] whois(1) out-of-bound access patch Message-ID: <20011005090727.A650@ringworld.oblivion.bg> In-Reply-To: <20011004134710.C31795@coffee.q9media.com>; from mike@FreeBSD.ORG on Thu, Oct 04, 2001 at 01:47:10PM -0400 References: <20011004121640.C1959@ringworld.oblivion.bg> <20011004121933.B31795@coffee.q9media.com> <200110041650.f94GoL10010161@xerxes.courtesan.com> <20011004134710.C31795@coffee.q9media.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 04, 2001 at 01:47:10PM -0400, Mike Barcroft wrote:
> Todd C. Miller <Todd.Miller@courtesan.com> writes:
> > In message <20011004121933.B31795@coffee.q9media.com>
> > so spake Mike Barcroft (mike):
> >
> > > Would you please test the attached patch and confirm that it solves
> > > the problem? If it does, I'll commit it today.
> >
> > I doubt that is sufficient as "buf" is treated as a NUL terminated
> > string in the calls to strstr(). Also note that it is not necessary
> > to copy the buffer each time as in the original patch. You can
> > only get a line w/o a newline as the last line before EOF.
The buffer is not copied each time, but only when a line w/o a newline
is found. In all other cases, we deal directly with what fgetln(3)
returns.
> We could always implement strnstr(). I think I prefer it to the
> malloc(3) the final line kludge.
strnstr() would not be enough; there are calls to strcspn(), strchr()
and s_asprintf() too, which treat buf as a null-terminated string.
I see no reason to introduce additional processing for *each* input
string, when all we need is to special-case the case of no newline.
The "kludge" is only invoked when a newline-less line is received,
which, as Todd Miller points out, is usually only the last single line.
In all other cases, there is no performance overhead.
On a side note, as Garrett Wollman kindly pointed out in a private
message, the calloc(3) call should probably be replaced by a malloc(3)
and zeroing only the last byte. See the attached revised patch.
G'luck,
Peter
--
You have, of course, just begun reading the sentence that you have just finished reading.
Index: src/usr.bin/whois/whois.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/whois/whois.c,v
retrieving revision 1.24
diff -u -r1.24 whois.c
--- src/usr.bin/whois/whois.c 2001/08/05 19:37:12 1.24
+++ src/usr.bin/whois/whois.c 2001/10/05 11:07:46
@@ -251,7 +251,7 @@
{
FILE *sfi, *sfo;
struct addrinfo *res2;
- char *buf, *nhost, *p;
+ char *abuf, *buf, *nhost, *p;
int i, nomatch, s;
size_t len;
@@ -275,7 +275,16 @@
nhost = NULL;
nomatch = 0;
while ((buf = fgetln(sfi, &len)) != NULL) {
- while (len && isspace(buf[len - 1]))
+ abuf = NULL;
+ if ((len == 0) || !isspace((unsigned char)buf[len - 1])) {
+ abuf = malloc(len + 1);
+ if (abuf == NULL)
+ err(1, "reallocating");
+ memcpy(abuf, buf, len);
+ abuf[len] = '\0';
+ buf = abuf;
+ }
+ while (len && isspace((unsigned char)buf[len - 1]))
buf[--len] = '\0';
if ((flags & WHOIS_RECURSE) && nhost == NULL) {
@@ -304,6 +313,7 @@
nomatch = 1;
}
printf("%s\n", buf);
+ free(abuf);
}
/* Do second lookup as needed. */
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?20011005090727.A650>