svn commit: r309639 - head/lib/libc/net

[Shawn Webb]

On Tue, Dec 06, 2016 at 06:50:34PM +0000, Gleb Smirnoff wrote:
> Author: glebius
> Date: Tue Dec  6 18:50:33 2016
> New Revision: 309639
> URL: https://svnweb.freebsd.org/changeset/base/309639
> 
> Log:
>   Fix possible buffer overflow(s) in link_ntoa(3).
>   
>   A specially crafted sockaddr_dl argument can trigger a static buffer overflow
>   in the libc library, with possibility to rewrite with arbitrary data following
>   static buffers that belong to other library functions.
>   
>   Reviewed by:	kib
>   Security:	FreeBSD-SA-16:37.libc
> 
> Modified:
>   head/lib/libc/net/linkaddr.c
> 
> Modified: head/lib/libc/net/linkaddr.c
> ==============================================================================
> --- head/lib/libc/net/linkaddr.c	Tue Dec  6 18:50:22 2016	(r309638)
> +++ head/lib/libc/net/linkaddr.c	Tue Dec  6 18:50:33 2016	(r309639)
> @@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
>  
>  #include <sys/types.h>
>  #include <sys/socket.h>
> +#include <net/if.h>
>  #include <net/if_dl.h>
>  #include <string.h>
>  
> @@ -122,31 +123,47 @@ char *
>  link_ntoa(const struct sockaddr_dl *sdl)
>  {
>  	static char obuf[64];
> -	char *out = obuf;
> -	int i;
> -	u_char *in = (u_char *)LLADDR(sdl);
> -	u_char *inlim = in + sdl->sdl_alen;
> -	int firsttime = 1;
> -
> -	if (sdl->sdl_nlen) {
> -		bcopy(sdl->sdl_data, obuf, sdl->sdl_nlen);
> -		out += sdl->sdl_nlen;
> -		if (sdl->sdl_alen)
> +	_Static_assert(sizeof(obuf) >= IFNAMSIZ + 20, "obuf is too small");
> +	char *out;
> +	const char *in, *inlim;
> +	int namelen, i, rem;
> +
> +	namelen = (sdl->sdl_nlen <= IFNAMSIZ) ? sdl->sdl_nlen : IFNAMSIZ;
> +
> +	out = obuf;
> +	rem = sizeof(obuf);
> +	if (namelen > 0) {
> +		bcopy(sdl->sdl_data, out, namelen);
> +		out += namelen;
> +		rem -= namelen;
> +		if (sdl->sdl_alen > 0) {
>  			*out++ = ':';
> +			rem--;
> +		}
>  	}
> -	while (in < inlim) {
> -		if (firsttime)
> -			firsttime = 0;
> -		else
> +
> +	in = (const char *)sdl->sdl_data + sdl->sdl_nlen;
> +	inlim = in + sdl->sdl_alen;
> +
> +	while (in < inlim && rem > 1) {
> +		if (in != (const char *)sdl->sdl_data + sdl->sdl_nlen) {
>  			*out++ = '.';
> +			rem--;
> +		}
>  		i = *in++;
>  		if (i > 0xf) {
> -			out[1] = hexlist[i & 0xf];
> +			if (rem < 3)
> +				break;
> +			*out++ = hexlist[i & 0xf];
>  			i >>= 4;
> -			out[0] = hexlist[i];
> -			out += 2;
> -		} else
>  			*out++ = hexlist[i];
> +			rem -= 2;
> +		} else {
> +			if (rem < 2)
> +				break;
> +			*out++ = hexlist[i];
> +			rem++;

rem++ is incorrect. It should be rem--. HardenedBSD has a fix here:

https://github.com/HardenedBSD/hardenedBSD/commit/fb823297fbced336b6beeeb624e2dc65b67aa0eb

> +		}
>  	}
>  	*out = 0;
>  	return (obuf);

Thanks,

-- 
Shawn Webb
Cofounder and Security Engineer
HardenedBSD

GPG Key ID:          0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20161207/e8f13d80/attachment.sig>