From owner-freebsd-security  Thu Aug 22 10:14:49 1996
Return-Path: owner-security
Received: (from root@localhost)
          by freefall.freebsd.org (8.7.5/8.7.3) id KAA00679
          for security-outgoing; Thu, 22 Aug 1996 10:14:49 -0700 (PDT)
Received: from cais.cais.com ([199.0.216.4])
          by freefall.freebsd.org (8.7.5/8.7.3) with SMTP id KAA00674
          for <freebsd-security@freebsd.org>; Thu, 22 Aug 1996 10:14:46 -0700 (PDT)
Received: from cais2.cais.com (cais2.cais.com [199.0.216.200]) by cais.cais.com (8.6.10/8.6.5) with ESMTP id NAA29726; Thu, 22 Aug 1996 13:13:57 -0400
Received: from localhost (jsdy@localhost) by cais2.cais.com (8.6.5/8.6.5) id NAA29924; Thu, 22 Aug 1996 13:13:55 -0400
Date: Thu, 22 Aug 1996 13:13:55 -0400
From: "Joseph S. D. Yao" <jsdy@cais.cais.com>
Message-Id: <199608221713.NAA29924@cais2.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
Sender: owner-security@freebsd.org
X-Loop: FreeBSD.org
Precedence: bulk

> 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