The following reply was made to PR bin/151664; it has been noted by GNATS.

From: Gleb Smirnoff <gleb...@freebsd.org>
To: Alexey Illarionov <littlesav...@rambler.ru>
Cc: freebsd-gnats-sub...@freebsd.org
Subject: Re: bin/151664: [PATCH] sbin/route/route.c: Incorrect array bounds
 checking
Date: Mon, 25 Oct 2010 18:12:06 +0400

 --G6nVm6DDWH/FONJq
 Content-Type: text/plain; charset=koi8-r
 Content-Disposition: inline
 
   Hello!
 
   Another patch, that also checks msglen even deeper down, when printing 
sockaddrs.
 
 -- 
 Totus tuus, Glebius.
 
 --G6nVm6DDWH/FONJq
 Content-Type: text/x-diff; charset=koi8-r
 Content-Disposition: attachment; filename="route.c.diff"
 
 Index: route.c
 ===================================================================
 --- route.c    (revision 213832)
 +++ route.c    (working copy)
 @@ -115,11 +115,11 @@
  static void   monitor(void);
  static const char     *netname(struct sockaddr *);
  static void   newroute(int, char **);
 -static void   pmsg_addrs(char *, int);
 -static void   pmsg_common(struct rt_msghdr *);
 +static void   pmsg_addrs(char *, int, size_t);
 +static void   pmsg_common(struct rt_msghdr *, size_t);
  static int    prefixlen(const char *);
  static void   print_getmsg(struct rt_msghdr *, int);
 -static void   print_rtmsg(struct rt_msghdr *, int);
 +static void   print_rtmsg(struct rt_msghdr *, size_t);
  static const char     *routename(struct sockaddr *);
  static int    rtmsg(int, int);
  static void   set_metric(char *, int);
 @@ -1306,7 +1306,7 @@
        "RTM_NEWMADDR: new multicast group membership on iface",
        "RTM_DELMADDR: multicast group membership removed from iface",
        "RTM_IFANNOUNCE: interface arrival/departure",
 -      0,
 +      "RTM_IEEE80211: IEEE 802.11 wireless event",
  };
  
  char metricnames[] =
 @@ -1325,7 +1325,7 @@
  "\1DST\2GATEWAY\3NETMASK\4GENMASK\5IFP\6IFA\7AUTHOR\010BRD";
  
  static void
 -print_rtmsg(struct rt_msghdr *rtm, int msglen __unused)
 +print_rtmsg(struct rt_msghdr *rtm, size_t msglen)
  {
        struct if_msghdr *ifm;
        struct ifa_msghdr *ifam;
 @@ -1342,13 +1342,22 @@
                    rtm->rtm_version);
                return;
        }
 -      if (msgtypes[rtm->rtm_type] != NULL)
 +      if (rtm->rtm_type < sizeof(msgtypes)/sizeof(msgtypes[0]))
                (void)printf("%s: ", msgtypes[rtm->rtm_type]);
        else
                (void)printf("#%d: ", rtm->rtm_type);
        (void)printf("len %d, ", rtm->rtm_msglen);
 +
 +#define       REQUIRE(x)      do {            \
 +      if (msglen < sizeof(x))         \
 +              goto badlen;            \
 +      else                            \
 +              msglen -= sizeof(x);    \
 +      } while (0)
 +
        switch (rtm->rtm_type) {
        case RTM_IFINFO:
 +              REQUIRE(struct if_msghdr);
                ifm = (struct if_msghdr *)rtm;
                (void) printf("if# %d, ", ifm->ifm_index);
                switch (ifm->ifm_data.ifi_link_state) {
 @@ -1364,23 +1373,26 @@
                }
                (void) printf("link: %s, flags:", state);
                bprintf(stdout, ifm->ifm_flags, ifnetflags);
 -              pmsg_addrs((char *)(ifm + 1), ifm->ifm_addrs);
 +              pmsg_addrs((char *)(ifm + 1), ifm->ifm_addrs, msglen);
                break;
        case RTM_NEWADDR:
        case RTM_DELADDR:
 +              REQUIRE(struct ifa_msghdr);
                ifam = (struct ifa_msghdr *)rtm;
                (void) printf("metric %d, flags:", ifam->ifam_metric);
                bprintf(stdout, ifam->ifam_flags, routeflags);
 -              pmsg_addrs((char *)(ifam + 1), ifam->ifam_addrs);
 +              pmsg_addrs((char *)(ifam + 1), ifam->ifam_addrs, msglen);
                break;
  #ifdef RTM_NEWMADDR
        case RTM_NEWMADDR:
        case RTM_DELMADDR:
 +              REQUIRE(struct ifma_msghdr);
                ifmam = (struct ifma_msghdr *)rtm;
 -              pmsg_addrs((char *)(ifmam + 1), ifmam->ifmam_addrs);
 +              pmsg_addrs((char *)(ifmam + 1), ifmam->ifmam_addrs, msglen);
                break;
  #endif
        case RTM_IFANNOUNCE:
 +              REQUIRE(struct if_announcemsghdr);
                ifan = (struct if_announcemsghdr *)rtm;
                (void) printf("if# %d, what: ", ifan->ifan_index);
                switch (ifan->ifan_what) {
 @@ -1401,8 +1413,13 @@
                (void) printf("pid: %ld, seq %d, errno %d, flags:",
                        (long)rtm->rtm_pid, rtm->rtm_seq, rtm->rtm_errno);
                bprintf(stdout, rtm->rtm_flags, routeflags);
 -              pmsg_common(rtm);
 +              pmsg_common(rtm, msglen);
        }
 +
 +      return;
 +
 +badlen:
 +      (void)printf("bad message length %u\n", msglen);
  }
  
  static void
 @@ -1490,7 +1507,7 @@
  #undef msec
  #define       RTA_IGN 
(RTA_DST|RTA_GATEWAY|RTA_NETMASK|RTA_IFP|RTA_IFA|RTA_BRD)
        if (verbose)
 -              pmsg_common(rtm);
 +              pmsg_common(rtm, msglen);
        else if (rtm->rtm_addrs &~ RTA_IGN) {
                (void) printf("sockaddrs: ");
                bprintf(stdout, rtm->rtm_addrs, addrnames);
 @@ -1500,17 +1517,21 @@
  }
  
  static void
 -pmsg_common(struct rt_msghdr *rtm)
 +pmsg_common(struct rt_msghdr *rtm, size_t msglen)
  {
        (void) printf("\nlocks: ");
        bprintf(stdout, rtm->rtm_rmx.rmx_locks, metricnames);
        (void) printf(" inits: ");
        bprintf(stdout, rtm->rtm_inits, metricnames);
 -      pmsg_addrs(((char *)(rtm + 1)), rtm->rtm_addrs);
 +      if (msglen > sizeof(struct rt_msghdr))
 +              pmsg_addrs(((char *)(rtm + 1)), rtm->rtm_addrs,
 +                  msglen - sizeof(struct rt_msghdr));
 +      else
 +              (void) fflush(stdout);
  }
  
  static void
 -pmsg_addrs(char *cp, int addrs)
 +pmsg_addrs(char *cp, int addrs, size_t len)
  {
        struct sockaddr *sa;
        int i;
 @@ -1524,8 +1545,17 @@
        (void) putchar('\n');
        for (i = 1; i != 0; i <<= 1)
                if (i & addrs) {
 +                      if (len < sizeof(struct sockaddr)) {
 +                              (void) printf("bad message length\n");
 +                              break;
 +                      }
                        sa = (struct sockaddr *)cp;
                        (void) printf(" %s", routename(sa));
 +                      if (len < SA_SIZE(sa)) {
 +                              (void) printf("bad message length\n");
 +                              break;
 +                      }
 +                      len -= SA_SIZE(sa);
                        cp += SA_SIZE(sa);
                }
        (void) putchar('\n');
 
 --G6nVm6DDWH/FONJq--
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"

Reply via email to