Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 7 Dec 2016 09:43:17 -0500
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Gleb Smirnoff <glebius@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r309639 - head/lib/libc/net
Message-ID:  <20161207144317.GB29174@mutt-hardenedbsd>
In-Reply-To: <201612061850.uB6IoY1U017268@repo.freebsd.org>
References:  <201612061850.uB6IoY1U017268@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--Bn2rw/3z4jIqBvZU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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
>=20
> Log:
>   Fix possible buffer overflow(s) in link_ntoa(3).
>  =20
>   A specially crafted sockaddr_dl argument can trigger a static buffer ov=
erflow
>   in the libc library, with possibility to rewrite with arbitrary data fo=
llowing
>   static buffers that belong to other library functions.
>  =20
>   Reviewed by:	kib
>   Security:	FreeBSD-SA-16:37.libc
>=20
> Modified:
>   head/lib/libc/net/linkaddr.c
>=20
> Modified: head/lib/libc/net/linkaddr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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$");
> =20
>  #include <sys/types.h>
>  #include <sys/socket.h>
> +#include <net/if.h>
>  #include <net/if_dl.h>
>  #include <string.h>
> =20
> @@ -122,31 +123,47 @@ char *
>  link_ntoa(const struct sockaddr_dl *sdl)
>  {
>  	static char obuf[64];
> -	char *out =3D obuf;
> -	int i;
> -	u_char *in =3D (u_char *)LLADDR(sdl);
> -	u_char *inlim =3D in + sdl->sdl_alen;
> -	int firsttime =3D 1;
> -
> -	if (sdl->sdl_nlen) {
> -		bcopy(sdl->sdl_data, obuf, sdl->sdl_nlen);
> -		out +=3D sdl->sdl_nlen;
> -		if (sdl->sdl_alen)
> +	_Static_assert(sizeof(obuf) >=3D IFNAMSIZ + 20, "obuf is too small");
> +	char *out;
> +	const char *in, *inlim;
> +	int namelen, i, rem;
> +
> +	namelen =3D (sdl->sdl_nlen <=3D IFNAMSIZ) ? sdl->sdl_nlen : IFNAMSIZ;
> +
> +	out =3D obuf;
> +	rem =3D sizeof(obuf);
> +	if (namelen > 0) {
> +		bcopy(sdl->sdl_data, out, namelen);
> +		out +=3D namelen;
> +		rem -=3D namelen;
> +		if (sdl->sdl_alen > 0) {
>  			*out++ =3D ':';
> +			rem--;
> +		}
>  	}
> -	while (in < inlim) {
> -		if (firsttime)
> -			firsttime =3D 0;
> -		else
> +
> +	in =3D (const char *)sdl->sdl_data + sdl->sdl_nlen;
> +	inlim =3D in + sdl->sdl_alen;
> +
> +	while (in < inlim && rem > 1) {
> +		if (in !=3D (const char *)sdl->sdl_data + sdl->sdl_nlen) {
>  			*out++ =3D '.';
> +			rem--;
> +		}
>  		i =3D *in++;
>  		if (i > 0xf) {
> -			out[1] =3D hexlist[i & 0xf];
> +			if (rem < 3)
> +				break;
> +			*out++ =3D hexlist[i & 0xf];
>  			i >>=3D 4;
> -			out[0] =3D hexlist[i];
> -			out +=3D 2;
> -		} else
>  			*out++ =3D hexlist[i];
> +			rem -=3D 2;
> +		} else {
> +			if (rem < 2)
> +				break;
> +			*out++ =3D hexlist[i];
> +			rem++;

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

https://github.com/HardenedBSD/hardenedBSD/commit/fb823297fbced336b6beeeb62=
4e2dc65b67aa0eb

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

Thanks,

--=20
Shawn Webb
Cofounder and Security Engineer
HardenedBSD

GPG Key ID:          0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE

--Bn2rw/3z4jIqBvZU
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJYSCACAAoJEGqEZY9SRW7u2HIQAJhX8ipRbKj519S+YzM8qe8s
0TU2Me3OVjYzH74DQm5OeS8y67G/mx4cjdwtUeSG59fUCT/Q7hXcGLeo3MV9BUAI
md/VWhD8e/0cvPC+CctbOHPrrTjOEtydjAF6Xx2osme5XLFFRiq4kw1bUq0KrupI
Ql/ILSktviMk8hyqgp+UgKZXytREQMn7nvPPXqdvduEEHr//Pj30sAD9FofFCuUE
B4MzCpKpRL14HL5GYv1ocOrPpKBG2rjF7q/nN3o3YO/kKKATa9o7iPyw0l6WMJC9
a39fC107vRwvkPoENApLsXhQv2Gcsh0jSMfnaFGw3BQw7UmFCeSqEoff20gCEp7r
ye4T0LARICm86SS5qSgPugMgiYNoCESKbiark06LlhNRrCelmXXuRKC1OwvJkRB5
V4beg2rfrmz9z5xyyfDoLbohn1gLcmwKqBLn8D5Du8ERX0RrYOHzFQLu50RUcXXy
gOV9cfzA7vUfpPS72YEqDcRbj7zBtfxLLyCM0e5Q+AagsM7MilUG2VWjpYg6FWuN
lUhNwwwTU/qwXXGvZaiaxauE3b8kqzPMYOg7CQMdqQKHL3uBp35YlaKTLgFYHJ1P
0veh56TSIEstWnYD36ekvyACUJ2rytwpI32Sn2Z4rgIyUN2Bnn+qnbgtHWnqpt3h
hZAOl/J1yH17uQD7Z36c
=Bnls
-----END PGP SIGNATURE-----

--Bn2rw/3z4jIqBvZU--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161207144317.GB29174>