svn commit: r253504 - head/sbin/route

[Hiroki Sato]

Ulrich Spörlein <uqs at FreeBSD.org> wrote
  in <20130724130046.GD9092 at 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> > ==============================================================================
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 = sizeof(struct sockaddr_dl);
uq> >  #endif
uq> >  	}
uq> > -	rtm_addrs |= which;
uq> > +	rtm_addrs |= (1 << idx);
uq> >  
uq> > -	switch (which) {
uq> > -	case RTA_DST:
uq> > -		sa = (struct sockaddr *)&so[RTAX_DST];
uq> > -		break;
uq> > -	case RTA_GATEWAY:
uq> > -		sa = (struct sockaddr *)&so[RTAX_GATEWAY];
uq> > -		break;
uq> > -	case RTA_NETMASK:
uq> > -		sa = (struct sockaddr *)&so[RTAX_NETMASK];
uq> > -		break;
uq> > -	case RTA_GENMASK:
uq> > -		sa = (struct sockaddr *)&so[RTAX_GENMASK];
uq> > -		break;
uq> > -	case RTA_IFA:
uq> > -		sa = (struct sockaddr *)&so[RTAX_IFA];
uq> > -		break;
uq> > -	case RTA_IFP:
uq> > -		sa = (struct sockaddr *)&so[RTAX_IFP];
uq> > -		break;
uq> > -	default:
uq> > +	if (idx > RTAX_MAX)
uq> >  		usage("internal error");
uq> > -		/*NOTREACHED*/
uq> > -	}
uq> > +	sa = (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 check
uq> for idx >= 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20130801/86b5010e/attachment.sig>