From owner-cvs-all@FreeBSD.ORG Sat Jan 31 12:17:57 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DAD4216A4CE; Sat, 31 Jan 2004 12:17:57 -0800 (PST) Received: from cirb503493.alcatel.com.au (c211-30-75-229.belrs2.nsw.optusnet.com.au [211.30.75.229]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5C73943D1F; Sat, 31 Jan 2004 12:17:55 -0800 (PST) (envelope-from PeterJeremy@optushome.com.au) Received: from cirb503493.alcatel.com.au (localhost.alcatel.com.au [127.0.0.1])i0VKHme3026598; Sun, 1 Feb 2004 07:17:48 +1100 (EST) (envelope-from jeremyp@cirb503493.alcatel.com.au) Received: (from jeremyp@localhost)i0VKHmCt026597; Sun, 1 Feb 2004 07:17:48 +1100 (EST) (envelope-from jeremyp) Date: Sun, 1 Feb 2004 07:17:48 +1100 From: Peter Jeremy To: Brooks Davis Message-ID: <20040131201747.GO908@cirb503493.alcatel.com.au> References: <200401270143.i0R1hEIO011023@repoman.freebsd.org> <20040129133342.GC19899@FreeBSD.org.ua> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200401270143.i0R1hEIO011023@repoman.freebsd.org> User-Agent: Mutt/1.4.1i cc: cvs-src@freebsd.org cc: src-committers@freebsd.org cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sbin/ifconfig ifconfig.c ifconfig.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 31 Jan 2004 20:17:58 -0000 On Mon, Jan 26, 2004 at 05:43:14PM -0800, Brooks Davis wrote: > brooks 2004/01/26 17:43:14 PST > > FreeBSD src repository > > Modified files: > sbin/ifconfig ifconfig.c ifconfig.h > Log: > Use IFNAMSIZ instead of a magic value for the length of an interface > name. > > Prevent the kernel from potentially overflowing the interface name > variable. The size argument of strlcpy is complex because the name is > not null-terminated in sdl_data. Based on this comment and a quick look at the code change, I don't believe this change is correct. The source argument to strlcpy(3) _must_ be NUL terminated - although strlcpy() will only copy the specified number of characters, it will traverse the source argument until it finds a NUL character so it can return the size of the source argument. In this case, sdl_data is not intentionally NUL terminated so the strlcpy() can scan forward over an arbitrary amount of process address space until stopped by a random NUL, SIGBUS or SIGSEGV. The latter two possibilities are undesirable :-). In this case, I believe the correct code is something like: memcpy(name, sdl->sdl_data, sizeof(name) < sdl->sdl_nlen ? sizeof(name)-1 : sdl->sdl_nlen); name[sizeof(name) < sdl->sdl_nlen ? sizeof(name)-1 : sdl->sdl_nlen] = '\0'; (strncpy could be used in place of memcpy). Peter