Date: Sun, 1 Feb 2004 07:17:48 +1100 From: Peter Jeremy <PeterJeremy@optushome.com.au> To: Brooks Davis <brooks@freebsd.org> Cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sbin/ifconfig ifconfig.c ifconfig.h Message-ID: <20040131201747.GO908@cirb503493.alcatel.com.au> In-Reply-To: <200401270143.i0R1hEIO011023@repoman.freebsd.org> References: <200401270143.i0R1hEIO011023@repoman.freebsd.org> <20040129133342.GC19899@FreeBSD.org.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040131201747.GO908>