From owner-svn-src-all@freebsd.org Sat Jan 27 23:21:47 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 08614EC9F69; Sat, 27 Jan 2018 23:21:47 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 7E8B17FB04; Sat, 27 Jan 2018 23:21:45 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 2944D10B8AB; Sun, 28 Jan 2018 10:21:43 +1100 (AEDT) Date: Sun, 28 Jan 2018 10:21:42 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Dimitry Andric cc: Ed Schouten , "Pedro F. Giffuni" , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r328492 - head/contrib/opie/libopie In-Reply-To: <7C471160-44B3-4EA6-8995-08A4EB4332A1@FreeBSD.org> Message-ID: <20180128093811.G4029@besplex.bde.org> References: <201801272216.w0RMGJwo057492@repo.freebsd.org> <7C471160-44B3-4EA6-8995-08A4EB4332A1@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.2 cv=YbvN30Zf c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=Hs2ZQaMOoJN6KwDMJ4AA:9 a=2D251b_JUCYupp0T:21 a=CEcq3IAuxX1zZp2m:21 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 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, 27 Jan 2018 23:21:47 -0000 On Sat, 27 Jan 2018, Dimitry Andric wrote: > On 27 Jan 2018, at 23:20, Ed Schouten wrote: >> >> 2018-01-27 23:16 GMT+01:00 Pedro F. Giffuni : >>> char host[sizeof(utmp.ut_host) + 1]; >>> insecure = 1; >>> >>> - strncpy(host, utmp.ut_host, sizeof(utmp.ut_host)); >>> - host[sizeof(utmp.ut_host)] = 0; >>> + strncpy(host, utmp.ut_host, sizeof(host)); >> >> Wait... This may access utmp.ut_host one byte past the end and no >> longer guarantees that host is null-terminated, right? > No, strncpy "copies at most len characters from src into dst". However, No, the change breaks the length so 1 byte past the end is accessed in implementations where ut_host is not guaranteed to be NUL terminated and the current instance of ut_host is not NUL terminated. > if the length of the source is equal to or greater than len, the > destination is *not* null terminated. This is likely why the > "host[sizeof(utmp.ut_host)] = 0;" statement was added. This is why that statement was there. This change is not even wrong under FreeBSD, since ut_host and several other fields are guaranteed to be NUL terminated in the FreeBSD implementation. The code was correct and portable and the change just breaks its portability. > In any case, this is why strlcpy exists. :) Using strlcpy() in libopie would be another good unportabilization. contrib/opie never uses strlc*() except in 1 place previously unportabilized in r208586. That at least fixed 2 bugs (2 related off by 1 errors in the code intended to avoid buffer overruns, with the result that buffer overruns were limited to 1 byte). It moved the style bugs by changing hacking on the source string to use of strlcpy(). Bruce