From owner-svn-src-all@freebsd.org Sun Jan 28 00:50:12 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 459BEECEA07 for ; Sun, 28 Jan 2018 00:50:12 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from sonic302-36.consmr.mail.gq1.yahoo.com (sonic302-36.consmr.mail.gq1.yahoo.com [98.137.68.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C16B482D4C for ; Sun, 28 Jan 2018 00:50:11 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1517100610; bh=P36D0DkgC2LqndVSYm4STDudviyLqsO7spJVCepvJhI=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From:Subject; b=J3RJsKSiRvscpm21v1mo7CGy+kU9KwNnUPhILJShzOUmZsylu8PYtUsqgsoQH7bLqhSZ0iOf8H7zwFSGsqjbil5p9ti6AQ029m4rqub+4mVWjhRNe6MEqDBHV+Wo5xm0Ia1sQhmhAcaRRGuSWC4119LUjwz9uL7AI7JBkRIie56VYGBQHCcWGIGi9b3Q1k8wxODdnwYsHxcSDJd1R/AhnnOgaPDZGLlyfLo8dQx8b2mdsLnKTTHqGhwc5vExgUJezpwVBzCI0VEkxWtH0SPU9hJNsrLODcf9GfMCOY0P/cNypPWo4fBUHMms/E0/vGruKhxD48AV1OJ3ciHIc7PP4Q== X-YMail-OSG: UcBmyo0VM1nmucI9exJVqET3QVSWO_X1hiWMgkw61vO1GdW2Y37HqTRVWOgQPfq Ex1y70V8fOvSBPUD0KHvGLjyKhdCX8NrVGa3MrrY8_.hJgW1UEUIh0Kxmehq4QO1KObfy9JAvYln zLmVmHrI5r__6aGYNbDAdagOIoyhMDFOoVKlQjrL31QizNNkAKupqvTrUO6kiN9BRwJFDL7AYqc5 668uXlcIgHHXKLapW_.BM_4Y2DexXacT1b9mnnF_GqwJrcsnborM73kgxxsYZElODnIHdxkKyHbE _ZqfrY5RunUDy8Nu9ewEmvXqHE8XMr_vMIefQKxWtCnL_nD6Psbhx_wNzB68HfxegI1622eF8tx7 7DsjViJZ_Y1XMgaIJfivRFp0ZtpI.jFx6FolBeJUmBO3sRG0Rz3vfJOwV2f5NEylMRHQuFnis7TW hr80iRQMND66yBulTHBQPjTD0OEnYbQAhxiFSTOR29gAV.24JDvXNTyhhQ918Qhb7nbffKcdMYBF l6bQ1PSY0qg-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic302.consmr.mail.gq1.yahoo.com with HTTP; Sun, 28 Jan 2018 00:50:10 +0000 Received: from smtp102.rhel.mail.gq1.yahoo.com (EHLO [192.168.0.8]) ([216.39.57.211]) by smtp409.mail.gq1.yahoo.com (JAMES SMTP Server ) with ESMTPA ID 9b32a739fd825fcb775a1f7b01996378; Sun, 28 Jan 2018 00:50:08 +0000 (UTC) Subject: Re: svn commit: r328492 - head/contrib/opie/libopie To: Bruce Evans , Dimitry Andric Cc: Ed Schouten , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201801272216.w0RMGJwo057492@repo.freebsd.org> <7C471160-44B3-4EA6-8995-08A4EB4332A1@FreeBSD.org> <20180128093811.G4029@besplex.bde.org> From: Pedro Giffuni Message-ID: <5c39c37d-8d0a-22e9-710b-2453e0dd4481@FreeBSD.org> Date: Sat, 27 Jan 2018 19:50:07 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180128093811.G4029@besplex.bde.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US 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: Sun, 28 Jan 2018 00:50:12 -0000 On 01/27/18 18:21, Bruce Evans wrote: > 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. > The main change is in the sizeof(). Regularly you should use the size of destination not the source, and apparently GCC8 decided there was something to check there. >> 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. > The change was done for portability to GCC, or at least to fix a warning there. >> 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(). > Looking in detail, upstream (which appears to have disappeared) does have the explicit NULL termination in our last import. For consistency and given that we already have a strlcpy in that code, we should use strlcpy() there. Every modern OS out there has strlcpy(3) and if not they can figure out what to do. Pedro.