Date: Sat, 14 May 2016 14:03:28 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Pedro F. Giffuni" <pfg@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r299709 - head/usr.sbin/timed/timed Message-ID: <20160514134125.Y6273@besplex.bde.org> In-Reply-To: <201605140242.u4E2gA7g005844@repo.freebsd.org> References: <201605140242.u4E2gA7g005844@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 14 May 2016, Pedro F. Giffuni wrote:
> Log:
> timed(8): Use strlcpy() for bounds checking.
>
> Prevent some theorical buffer overruns reported by Coverity.
> Cleanup a use of gethostname() while here.
>
> CID: 1006713, 1011166, 1011167, 1011168,
This has minor unimprovements except it breaks the error checking for
gethostname().
> ...
> Modified: head/usr.sbin/timed/timed/timed.c
> ==============================================================================
> --- head/usr.sbin/timed/timed/timed.c Sat May 14 01:12:23 2016 (r299708)
> +++ head/usr.sbin/timed/timed/timed.c Sat May 14 02:42:09 2016 (r299709)
> @@ -196,7 +196,7 @@ main(int argc, char *argv[])
> if (goodgroup != NULL || goodhosts != NULL)
> Mflag = 1;
>
> - if (gethostname(hostname, sizeof(hostname) - 1) < 0)
> + if (gethostname(hostname, sizeof(hostname)) < 0)
> err(1, "gethostname");
> self.l_bak = &self;
> self.l_fwd = &self;
gethostname() returns a non-NUL terminated buffer with no error if the
non-terminated array fits exactly.
The old code carefully arranges for NUL termination if the system's
hostname has length sizeof(hostname) - 1 (although the syscall doesn't
give termination) and an error if the system's hostname has length
sizeof(hostname).
The new code gives a non-NUL-terminated buffer if the system's
hostname has length sizeof(hostname). Buffer overruns soon occur in
code that expects the hostname variable to be a string.
The overrun probably can't occur in practice, since the hostname variable
has the current maximum size, unless someone enlarges {HOST_NAME_MAX}.
Enlarging it would break old applications that use MAXHOSTNAMELEN instead
of {HOST_NAME_MAX} and have buggy error handling.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160514134125.Y6273>