Ulrich Spörlein <u...@freebsd.org> wrote
  in <20130724130046.gd9...@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

Attachment: pgp_sN0Y0jTtV.pgp
Description: PGP signature

Reply via email to