Date: Thu, 22 Aug 1996 13:13:55 -0400 From: "Joseph S. D. Yao" <jsdy@cais.cais.com> To: bugtraq@netspace.org, davem@iss.net, deraadt@theos.com, freebsd-security@freebsd.org, linux-security@tarsier.cv.nrao.edu Subject: Re: [linux-security] rwhod buffer overflow Message-ID: <199608221713.NAA29924@cais2.cais.com>
next in thread | raw e-mail | index | archive | help
> There is a remote buffer overflow in the path variable in rwhod.c in the
> line: (void) sprintf(path, "whod.%s", wd.wd_hostname);
...
> I would suggest prior to the sprintf line you add something to the effect:
> if(strlen(wd.wd_hostname) >= sizeof(wd.wd_hostname)) {
> syslog(LOG_WARNING, "possible hostname overflow attack apparently from %x",
> from.sin_addr);
> continue;
> }
You might also wish to modify the sprintf() as follows. Just because
wd_hostname fits into wd doesn't mean (in some future revision) that it
will fit into path.
static char path_prefix[] = "whod.";
(void) sprintf(path, "%s%.*s", path_prefix,
sizeof(path) - sizeof(path_prefix), wd.wd_hostname);
The above assumes that path is an array, rather than a pointer: I
haven't looked. If it's a pointer, then change sizeof(path) to the
defined constant that reliably defines the size of the array to which
path points. This also neatly accounts for the terminating NUL,
because that is measured in sizeof(path_prefix), but not copied over by
"%s" in the sprintf() call.
Yes, this will truncate some LONG host names. A better algorithm would
find the combined lengths of the path_prefix + the hostname, allocate a
buffer at least that long + 1 (if not already allocated), die or skip
the host if the alloc fails (so many programs forget to check!!!), and
then do the copy, freeing the buffer when [if] it's no longer being
used. But that's a bigger patch than the above. [;-\]
Joe Yao jsdy@cais.com - Joseph S. D. Yao
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199608221713.NAA29924>