From owner-svn-src-all@freebsd.org Sat May 14 04:03:33 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 34CBCB36575; Sat, 14 May 2016 04:03:33 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id F39821650; Sat, 14 May 2016 04:03:32 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-106-149-109.carlnfd1.nsw.optusnet.com.au (c122-106-149-109.carlnfd1.nsw.optusnet.com.au [122.106.149.109]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 1F4B11A5CBD; Sat, 14 May 2016 14:03:31 +1000 (AEST) Date: Sat, 14 May 2016 14:03:28 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" 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 In-Reply-To: <201605140242.u4E2gA7g005844@repo.freebsd.org> Message-ID: <20160514134125.Y6273@besplex.bde.org> References: <201605140242.u4E2gA7g005844@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=c+ZWOkJl c=1 sm=1 tr=0 a=R/f3m204ZbWUO/0rwPSMPw==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=OiLl4YRflpIkE0SgtRgA:9 a=ZVJtu2ht00FAafAy:21 a=mR92S3R_96XxYTox:21 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 May 2016 04:03:33 -0000 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