From owner-svn-src-head@freebsd.org Sat Aug 1 23:51:59 2015 Return-Path: Delivered-To: svn-src-head@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 006CF9B1297 for ; Sat, 1 Aug 2015 23:51:59 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from nm10-vm0.bullet.mail.bf1.yahoo.com (nm10-vm0.bullet.mail.bf1.yahoo.com [98.139.213.147]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A52361423 for ; Sat, 1 Aug 2015 23:51:58 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1438473110; bh=5VOraS6nx1J7EvjedJF1xoO2Ni8nec9XbExFM64BkRM=; h=Date:From:To:Subject:References:In-Reply-To:From:Subject; b=rhJrUIJ7FjJPpbMocp5sadPpWTr3SLxXjAAAkdwjigPEE2UvA725RvtbKZ4/loCpboNnsI2J6FLFroVYxhxjZK2EfaUAls5gCsrjuBRxjUsstgcIRDKK+W/8iFojjU3jtlIQ89vQueQnnGMkQqzrE9dh/kmTeNRefLQjjncjXXWYGwPbYoCI1XC5IcLbEVHv5bUBlFeBSpL8afVsgbEzy12c9QspCsE/ZOmO62g9HWyQY16XBWMwccrtXvkuBWgi5jcLzl69VFTV4+9KDF2JtKfXjhRSpHwlh4GDwzFx91gDaysE1PxxldsUNcuLV4bQrrFUoNBh4KsezrWHZfkZMQ== Received: from [98.139.215.142] by nm10.bullet.mail.bf1.yahoo.com with NNFMP; 01 Aug 2015 23:51:50 -0000 Received: from [98.139.211.206] by tm13.bullet.mail.bf1.yahoo.com with NNFMP; 01 Aug 2015 23:51:50 -0000 Received: from [127.0.0.1] by smtp215.mail.bf1.yahoo.com with NNFMP; 01 Aug 2015 23:51:50 -0000 X-Yahoo-Newman-Id: 474349.33464.bm@smtp215.mail.bf1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 4tOhtmMVM1k7tnuPIwJVe0n02D4c0MSnKPpli2n6CxlgRwK Da56zqwv_C21zfLGbYjqD0FEif2lg3A1MLxd1IZ0BJW_ZVGZgS0.WlK0fSPk gmSIxEkRrwUiGffX7usnyDa9BjqMCAdtBzxCKAwCRwScsgRlIMdmGDjYbo4R IQ1CEFV207fBsuAqLVMFd0qBD9W.EOcamR4ojpRtyyMqobjWdvA29jnJTK3T DV8YcyLu5vgqfXd8bmV0GjT0DVGQ2tZrAR8BzDYLcNR1tUp5Vu1M5fJtnEIc r3_CSiE5DHiLyn58oZwG_msnn90fpUY9cLsjbhcyDhyJXMB6W8uM57BVJG4n 3g0nTa017Q9XY6BpJvFHJlOA6Vwhb9mavcGLfsq5kgoEFUYqp6KXdRUL0hAq ktMn8KYAm42SsngQPGXh72iAYJRfZ0wVq4JfLZIT6ynSmRS2gjE7S8WrV8N_ AFDAmp5Jd6zy6V89lQCy8h9DuJ5Cw.m3_yi60pU44e6UJicCMxGx5OeKg2lt ZmtDfx28_t.hP5AuKhP8pOmzXoekjgnNw X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf Message-ID: <55BD5B9F.2030906@FreeBSD.org> Date: Sat, 01 Aug 2015 18:51:59 -0500 From: Pedro Giffuni Organization: FreeBSD Project User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Eric van Gyzen , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r286144 - head/usr.bin/wall References: <201508010129.t711TuAv036881@repo.freebsd.org> <55BCD8E3.3070607@FreeBSD.org> <55BCDF2D.8000700@FreeBSD.org> In-Reply-To: <55BCDF2D.8000700@FreeBSD.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Aug 2015 23:51:59 -0000 On 01/08/2015 10:01 a.m., Pedro Giffuni wrote: > > > On 08/01/15 09:34, Eric van Gyzen wrote: >> On 7/31/15 8:29 PM, Pedro F. Giffuni wrote: >>> Author: pfg >>> Date: Sat Aug 1 01:29:55 2015 >>> New Revision: 286144 >>> URL: https://svnweb.freebsd.org/changeset/base/286144 >>> >>> Log: >>> Buffer overflow in wall(1). >>> >>> Revert r286102 and apply a cleaner fix. >>> Tested for overflows by FORTIFY_SOURCE GSoC (with clang). >>> >>> Suggested by: bde >>> Reviewed by: Oliver Pinter >>> Tested by: Oliver Pinter >>> MFC after: 3 days >>> >>> Modified: >>> head/usr.bin/wall/ttymsg.c >>> >>> Modified: head/usr.bin/wall/ttymsg.c >>> ============================================================================== >>> >>> >>> --- head/usr.bin/wall/ttymsg.c Fri Jul 31 23:40:18 2015 (r286143) >>> +++ head/usr.bin/wall/ttymsg.c Sat Aug 1 01:29:55 2015 (r286144) >>> @@ -62,7 +62,7 @@ ttymsg(struct iovec *iov, int iovcnt, co >>> struct iovec localiov[7]; >>> ssize_t left, wret; >>> int cnt, fd; >>> - char device[MAXNAMLEN] = _PATH_DEV; >>> + char device[MAXNAMLEN]; >>> static char errbuf[1024]; >>> char *p; >>> int forked; >>> @@ -71,8 +71,9 @@ ttymsg(struct iovec *iov, int iovcnt, co >>> if (iovcnt > (int)(sizeof(localiov) / sizeof(localiov[0]))) >>> return ("too many iov's (change code in wall/ttymsg.c)"); >>> >>> - strlcat(device, line, sizeof(device)); >>> + strlcpy(device, _PATH_DEV, sizeof(device)); >>> p = device + sizeof(_PATH_DEV) - 1; >>> + strlcpy(p, line, sizeof(device) - sizeof(_PATH_DEV)); >> >> You're probably already sick of this change, but I would encourage this >> instead: >> >> strlcat(device, line, sizeof(device)); >> > > It looks exactly like the code I just replaced ;). > >> Your current code works, and it's even more efficient than strlcat. > Actually it looks like there may be some consensus towards reverting this as the previous version was more readable. Since the code still works better than when there was an overflow, let's leave it for a couple of days, maybe interested developers will come with a better change. Pedro.