Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 01 Aug 2013 13:51:56 +0900 (JST)
From:      Hiroki Sato <hrs@FreeBSD.org>
To:        uqs@FreeBSD.org
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r253504 - head/sbin/route
Message-ID:  <20130801.135156.1110859998150839575.hrs@allbsd.org>
In-Reply-To: <20130724130046.GD9092@acme.spoerlein.net>
References:  <201307201646.r6KGkpM6054344@svn.freebsd.org> <20130724130046.GD9092@acme.spoerlein.net>

next in thread | previous in thread | raw e-mail | index | archive | help
----Security_Multipart(Thu_Aug__1_13_51_56_2013_060)--
Content-Type: Text/Plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable

Ulrich Sp=F6rlein <uqs@FreeBSD.org> wrote
  in <20130724130046.GD9092@acme.spoerlein.net>:

uq> On Sat, 2013-07-20 at 16:46:51 +0000, Hiroki Sato wrote:
uq> > Author: hrs
uq> > Date: Sat Jul 20 16:46:51 2013
uq> > New Revision: 253504
uq> > URL: http://svnweb.freebsd.org/changeset/base/253504
uq> > =

uq> > Log:
uq> >   - Simplify getaddr() and print_getmsg() by using RTAX_* instead=
 of RTA_*
uq> >     as the argument.
uq> >   - Reduce unnecessary loop in print_getmsg().
uq> > =

uq> > Modified:
uq> >   head/sbin/route/route.c
uq> > =

uq> > Modified: head/sbin/route/route.c
uq> > =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
uq> > --- head/sbin/route/route.c	Sat Jul 20 15:58:43 2013	(r253503)
uq> > +++ head/sbin/route/route.c	Sat Jul 20 16:46:51 2013	(r253504)
uq> > @@ -1105,7 +1105,7 @@ inet6_makenetandmask(struct sockaddr_in6
uq> >   * returning 1 if a host address, 0 if a network address.
uq> >   */
uq> >  static int
uq> > -getaddr(int which, char *str, struct hostent **hpp, int nrflags)=

uq> > +getaddr(int idx, char *str, struct hostent **hpp, int nrflags)
uq> >  {
uq> >  	struct sockaddr *sa;
uq> >  #if defined(INET)
uq> > @@ -1130,36 +1130,16 @@ getaddr(int which, char *str, struct hos
uq> >  		aflen =3D sizeof(struct sockaddr_dl);
uq> >  #endif
uq> >  	}
uq> > -	rtm_addrs |=3D which;
uq> > +	rtm_addrs |=3D (1 << idx);
uq> >  =

uq> > -	switch (which) {
uq> > -	case RTA_DST:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_DST];
uq> > -		break;
uq> > -	case RTA_GATEWAY:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_GATEWAY];
uq> > -		break;
uq> > -	case RTA_NETMASK:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_NETMASK];
uq> > -		break;
uq> > -	case RTA_GENMASK:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_GENMASK];
uq> > -		break;
uq> > -	case RTA_IFA:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_IFA];
uq> > -		break;
uq> > -	case RTA_IFP:
uq> > -		sa =3D (struct sockaddr *)&so[RTAX_IFP];
uq> > -		break;
uq> > -	default:
uq> > +	if (idx > RTAX_MAX)
uq> >  		usage("internal error");
uq> > -		/*NOTREACHED*/
uq> > -	}
uq> > +	sa =3D (struct sockaddr *)&so[idx];
uq> =

uq> Coverity Scan flags this as an out-of-bounds write. RTAX_MAX is 8, =
so
uq> idx can be up to 8 (inclusive) in the check above. Do you want to c=
heck
uq> for idx >=3D RTAX_MAX maybe? idx is also signed ...
uq> =

uq> Coverity CID is 1054779, btw.

 Sorry for the delay.  Thank you for pointing out it.  Yes, the check
 was wrong by one.  Fixed in r253852.

-- Hiroki

----Security_Multipart(Thu_Aug__1_13_51_56_2013_060)--
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (FreeBSD)

iEYEABECAAYFAlH56WwACgkQTyzT2CeTzy25QwCgqPupVuyMx+IPixqiigNUL9Kc
4LwAoL7riDpIAhcih8VMIXaOwbCVoSyw
=B9Ox
-----END PGP SIGNATURE-----

----Security_Multipart(Thu_Aug__1_13_51_56_2013_060)----



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