cvs commit: src/sbin/ifconfig ifconfig.c ifconfig.h

[Peter Jeremy]

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