svn commit: r299709 - head/usr.sbin/timed/timed

[Bruce Evans]

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