From owner-svn-src-all@FreeBSD.ORG Thu Aug 1 04:52:23 2013 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 4C85BF03; Thu, 1 Aug 2013 04:52:23 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from mail.allbsd.org (gatekeeper.allbsd.org [IPv6:2001:2f0:104:e001::32]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 003212758; Thu, 1 Aug 2013 04:52:21 +0000 (UTC) Received: from alph.d.allbsd.org (p2049-ipbf1102funabasi.chiba.ocn.ne.jp [122.26.101.49]) (authenticated bits=128) by mail.allbsd.org (8.14.5/8.14.5) with ESMTP id r714q3V9048429 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 1 Aug 2013 13:52:13 +0900 (JST) (envelope-from hrs@FreeBSD.org) Received: from localhost (localhost [IPv6:::1]) (authenticated bits=0) by alph.d.allbsd.org (8.14.5/8.14.5) with ESMTP id r714q2uX057545; Thu, 1 Aug 2013 13:52:02 +0900 (JST) (envelope-from hrs@FreeBSD.org) Date: Thu, 01 Aug 2013 13:51:56 +0900 (JST) Message-Id: <20130801.135156.1110859998150839575.hrs@allbsd.org> To: uqs@FreeBSD.org Subject: Re: svn commit: r253504 - head/sbin/route From: Hiroki Sato In-Reply-To: <20130724130046.GD9092@acme.spoerlein.net> References: <201307201646.r6KGkpM6054344@svn.freebsd.org> <20130724130046.GD9092@acme.spoerlein.net> X-PGPkey-fingerprint: BDB3 443F A5DD B3D0 A530 FFD7 4F2C D3D8 2793 CF2D X-Mailer: Mew version 6.5 on Emacs 24.3 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Multipart/Signed; protocol="application/pgp-signature"; micalg=pgp-sha1; boundary="--Security_Multipart(Thu_Aug__1_13_51_56_2013_060)--" Content-Transfer-Encoding: 7bit X-Virus-Scanned: clamav-milter 0.97.4 at gatekeeper.allbsd.org X-Virus-Status: Clean X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (mail.allbsd.org [133.31.130.32]); Thu, 01 Aug 2013 13:52:13 +0900 (JST) X-Spam-Status: No, score=-90.0 required=13.0 tests=CONTENT_TYPE_PRESENT, DIRECTOCNDYN,DYN_PBL,MIMEQENC,QENCPTR1,QENCPTR2,RCVD_IN_PBL,SPF_SOFTFAIL, USER_IN_WHITELIST autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on gatekeeper.allbsd.org Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Aug 2013 04:52:23 -0000 ----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 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)----