Author: pkelsey Date: Sun Apr 16 19:17:10 2017 New Revision: 317035 URL: https://svnweb.freebsd.org/changeset/base/317035
Log: Fix userland tools that don't check the format of routing socket messages before accessing message fields that may not be present, removing dead/duplicate/misleading code along the way. Document the message format for each routing socket message in route.h. Fix a bug in usr.bin/netstat introduced in r287351 that resulted in pointer computation with essentially random 16-bit offsets and dereferencing of the results. Reviewed by: ae MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D10330 Modified: head/contrib/traceroute/findsaddr-socket.c head/sbin/route/route.c head/sbin/routed/table.c head/sys/net/route.h head/usr.bin/netstat/route.c head/usr.sbin/arp/arp.c head/usr.sbin/ndp/ndp.c head/usr.sbin/rarpd/rarpd.c head/usr.sbin/route6d/route6d.c Modified: head/contrib/traceroute/findsaddr-socket.c ============================================================================== --- head/contrib/traceroute/findsaddr-socket.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/contrib/traceroute/findsaddr-socket.c Sun Apr 16 19:17:10 2017 (r317035) @@ -156,7 +156,8 @@ findsaddr(register const struct sockaddr return (errbuf); } - } while (rp->rtm_seq != seq || rp->rtm_pid != pid); + } while (rp->rtm_type != RTM_GET || rp->rtm_seq != seq || + rp->rtm_pid != pid); close(s); Modified: head/sbin/route/route.c ============================================================================== --- head/sbin/route/route.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/sbin/route/route.c Sun Apr 16 19:17:10 2017 (r317035) @@ -1497,10 +1497,7 @@ rtmsg(int cmd, int flags, int fib) #define NEXTADDR(w, u) \ if (rtm_addrs & (w)) { \ - l = (((struct sockaddr *)&(u))->sa_len == 0) ? \ - sizeof(long) : \ - 1 + ((((struct sockaddr *)&(u))->sa_len - 1) \ - | (sizeof(long) - 1)); \ + l = SA_SIZE(&(u)); \ memmove(cp, (char *)&(u), l); \ cp += l; \ if (verbose) \ @@ -1564,7 +1561,8 @@ rtmsg(int cmd, int flags, int fib) do { l = read(s, (char *)&m_rtmsg, sizeof(m_rtmsg)); } while (l > 0 && stop_read == 0 && - (rtm.rtm_seq != rtm_seq || rtm.rtm_pid != pid)); + (rtm.rtm_type != RTM_GET || rtm.rtm_seq != rtm_seq || + rtm.rtm_pid != pid)); if (stop_read != 0) { warnx("read from routing socket timed out"); return (-1); @@ -1706,10 +1704,13 @@ print_rtmsg(struct rt_msghdr *rtm, size_ break; default: - printf("pid: %ld, seq %d, errno %d, flags:", - (long)rtm->rtm_pid, rtm->rtm_seq, rtm->rtm_errno); - printb(rtm->rtm_flags, routeflags); - pmsg_common(rtm, msglen); + if (rtm->rtm_type <= RTM_RESOLVE) { + printf("pid: %ld, seq %d, errno %d, flags:", + (long)rtm->rtm_pid, rtm->rtm_seq, rtm->rtm_errno); + printb(rtm->rtm_flags, routeflags); + pmsg_common(rtm, msglen); + } else + printf("type: %u, len: %zu\n", rtm->rtm_type, msglen); } return; Modified: head/sbin/routed/table.c ============================================================================== --- head/sbin/routed/table.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/sbin/routed/table.c Sun Apr 16 19:17:10 2017 (r317035) @@ -1233,6 +1233,15 @@ read_rt(void) if (m.r.rtm.rtm_type <= RTM_CHANGE) strp += sprintf(strp," from pid %d",m.r.rtm.rtm_pid); + /* + * Only messages that use the struct rt_msghdr format are + * allowed beyond this point. + */ + if (m.r.rtm.rtm_type > RTM_RESOLVE) { + trace_act("ignore %s", str); + continue; + } + rt_xaddrs(&info, m.r.addrs, &m.r.addrs[RTAX_MAX], m.r.rtm.rtm_addrs); Modified: head/sys/net/route.h ============================================================================== --- head/sys/net/route.h Sun Apr 16 19:12:07 2017 (r317034) +++ head/sys/net/route.h Sun Apr 16 19:17:10 2017 (r317035) @@ -265,25 +265,35 @@ struct rt_msghdr { /* * Message types. + * + * The format for each message is annotated below using the following + * identifiers: + * + * (1) struct rt_msghdr + * (2) struct ifa_msghdr + * (3) struct if_msghdr + * (4) struct ifma_msghdr + * (5) struct if_announcemsghdr + * */ -#define RTM_ADD 0x1 /* Add Route */ -#define RTM_DELETE 0x2 /* Delete Route */ -#define RTM_CHANGE 0x3 /* Change Metrics or flags */ -#define RTM_GET 0x4 /* Report Metrics */ -#define RTM_LOSING 0x5 /* Kernel Suspects Partitioning */ -#define RTM_REDIRECT 0x6 /* Told to use different route */ -#define RTM_MISS 0x7 /* Lookup failed on this address */ -#define RTM_LOCK 0x8 /* fix specified metrics */ +#define RTM_ADD 0x1 /* (1) Add Route */ +#define RTM_DELETE 0x2 /* (1) Delete Route */ +#define RTM_CHANGE 0x3 /* (1) Change Metrics or flags */ +#define RTM_GET 0x4 /* (1) Report Metrics */ +#define RTM_LOSING 0x5 /* (1) Kernel Suspects Partitioning */ +#define RTM_REDIRECT 0x6 /* (1) Told to use different route */ +#define RTM_MISS 0x7 /* (1) Lookup failed on this address */ +#define RTM_LOCK 0x8 /* (1) fix specified metrics */ /* 0x9 */ /* 0xa */ -#define RTM_RESOLVE 0xb /* req to resolve dst to LL addr */ -#define RTM_NEWADDR 0xc /* address being added to iface */ -#define RTM_DELADDR 0xd /* address being removed from iface */ -#define RTM_IFINFO 0xe /* iface going up/down etc. */ -#define RTM_NEWMADDR 0xf /* mcast group membership being added to if */ -#define RTM_DELMADDR 0x10 /* mcast group membership being deleted */ -#define RTM_IFANNOUNCE 0x11 /* iface arrival/departure */ -#define RTM_IEEE80211 0x12 /* IEEE80211 wireless event */ +#define RTM_RESOLVE 0xb /* (1) req to resolve dst to LL addr */ +#define RTM_NEWADDR 0xc /* (2) address being added to iface */ +#define RTM_DELADDR 0xd /* (2) address being removed from iface */ +#define RTM_IFINFO 0xe /* (3) iface going up/down etc. */ +#define RTM_NEWMADDR 0xf /* (4) mcast group membership being added to if */ +#define RTM_DELMADDR 0x10 /* (4) mcast group membership being deleted */ +#define RTM_IFANNOUNCE 0x11 /* (5) iface arrival/departure */ +#define RTM_IEEE80211 0x12 /* (5) IEEE80211 wireless event */ /* * Bitmask values for rtm_inits and rmx_locks. @@ -342,11 +352,10 @@ struct rt_addrinfo { * This macro returns the size of a struct sockaddr when passed * through a routing socket. Basically we round up sa_len to * a multiple of sizeof(long), with a minimum of sizeof(long). - * The check for a NULL pointer is just a convenience, probably never used. * The case sa_len == 0 should only apply to empty structures. */ #define SA_SIZE(sa) \ - ( (!(sa) || ((struct sockaddr *)(sa))->sa_len == 0) ? \ + ( (((struct sockaddr *)(sa))->sa_len == 0) ? \ sizeof(long) : \ 1 + ( (((struct sockaddr *)(sa))->sa_len - 1) | (sizeof(long) - 1) ) ) Modified: head/usr.bin/netstat/route.c ============================================================================== --- head/usr.bin/netstat/route.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/usr.bin/netstat/route.c Sun Apr 16 19:17:10 2017 (r317035) @@ -360,9 +360,10 @@ p_rtentry_sysctl(const char *name, struc xo_open_instance(name); sa = (struct sockaddr *)(rtm + 1); for (i = 0; i < RTAX_MAX; i++) { - if (rtm->rtm_addrs & (1 << i)) + if (rtm->rtm_addrs & (1 << i)) { addr[i] = sa; - sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa)); + sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa)); + } } protrusion = p_sockaddr("destination", addr[RTAX_DST], Modified: head/usr.sbin/arp/arp.c ============================================================================== --- head/usr.sbin/arp/arp.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/usr.sbin/arp/arp.c Sun Apr 16 19:17:10 2017 (r317035) @@ -404,7 +404,7 @@ set(int argc, char **argv) * the prefix route covering the local end of the * PPP link should be returned, on which ARP applies. */ - rtm = rtmsg(RTM_GET, dst, &sdl_m); + rtm = rtmsg(RTM_GET, dst, NULL); if (rtm == NULL) { xo_warn("%s", host); return (1); @@ -466,7 +466,6 @@ delete(char *host) struct sockaddr_in *addr, *dst; struct rt_msghdr *rtm; struct sockaddr_dl *sdl; - struct sockaddr_dl sdl_m; dst = getaddr(host); if (dst == NULL) @@ -477,17 +476,8 @@ delete(char *host) */ flags &= ~RTF_ANNOUNCE; - /* - * setup the data structure to notify the kernel - * it is the ARP entry the RTM_GET is interested - * in - */ - bzero(&sdl_m, sizeof(sdl_m)); - sdl_m.sdl_len = sizeof(sdl_m); - sdl_m.sdl_family = AF_LINK; - for (;;) { /* try twice */ - rtm = rtmsg(RTM_GET, dst, &sdl_m); + rtm = rtmsg(RTM_GET, dst, NULL); if (rtm == NULL) { xo_warn("%s", host); return (1); @@ -511,7 +501,7 @@ delete(char *host) } /* - * Regualar entry delete failed, now check if there + * Regular entry delete failed, now check if there * is a proxy-arp entry to remove. */ if (flags & RTF_ANNOUNCE) { @@ -815,7 +805,8 @@ doit: } do { l = read(s, (char *)&m_rtmsg, sizeof(m_rtmsg)); - } while (l > 0 && (rtm->rtm_seq != seq || rtm->rtm_pid != pid)); + } while (l > 0 && (rtm->rtm_type != cmd || rtm->rtm_seq != seq || + rtm->rtm_pid != pid)); if (l < 0) xo_warn("read from routing socket"); return (rtm); Modified: head/usr.sbin/ndp/ndp.c ============================================================================== --- head/usr.sbin/ndp/ndp.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/usr.sbin/ndp/ndp.c Sun Apr 16 19:17:10 2017 (r317035) @@ -888,7 +888,8 @@ doit: } do { l = read(s, (char *)&m_rtmsg, sizeof(m_rtmsg)); - } while (l > 0 && (rtm->rtm_seq != seq || rtm->rtm_pid != pid)); + } while (l > 0 && (rtm->rtm_type != cmd || rtm->rtm_seq != seq || + rtm->rtm_pid != pid)); if (l < 0) (void) fprintf(stderr, "ndp: read from routing socket: %s\n", strerror(errno)); Modified: head/usr.sbin/rarpd/rarpd.c ============================================================================== --- head/usr.sbin/rarpd/rarpd.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/usr.sbin/rarpd/rarpd.c Sun Apr 16 19:17:10 2017 (r317035) @@ -755,7 +755,8 @@ update_arptab(u_char *ep, in_addr_t ipad } do { cc = read(r, rt, sizeof(rtmsg)); - } while (cc > 0 && (rt->rtm_seq != seq || rt->rtm_pid != pid)); + } while (cc > 0 && (rt->rtm_type != RTM_GET || rt->rtm_seq != seq || + rt->rtm_pid != pid)); if (cc == -1) { logmsg(LOG_ERR, "rtmsg get read: %m"); close(r); @@ -803,7 +804,8 @@ update_arptab(u_char *ep, in_addr_t ipad } do { cc = read(r, rt, sizeof(rtmsg)); - } while (cc > 0 && (rt->rtm_seq != seq || rt->rtm_pid != pid)); + } while (cc > 0 && (rt->rtm_type != RTM_ADD || rt->rtm_seq != seq || + rt->rtm_pid != pid)); close(r); if (cc == -1) { logmsg(LOG_ERR, "rtmsg add read: %m"); Modified: head/usr.sbin/route6d/route6d.c ============================================================================== --- head/usr.sbin/route6d/route6d.c Sun Apr 16 19:12:07 2017 (r317034) +++ head/usr.sbin/route6d/route6d.c Sun Apr 16 19:17:10 2017 (r317035) @@ -1768,14 +1768,23 @@ rtrecv(void) break; default: rtm = (struct rt_msghdr *)(void *)p; - addrs = rtm->rtm_addrs; - q = (char *)(rtm + 1); if (rtm->rtm_version != RTM_VERSION) { trace(1, "unexpected rtmsg version %d " "(should be %d)\n", rtm->rtm_version, RTM_VERSION); continue; } + /* + * Only messages that use the struct rt_msghdr + * format are allowed beyond this point. + */ + if (rtm->rtm_type > RTM_RESOLVE) { + trace(1, "rtmsg type %d ignored\n", + rtm->rtm_type); + continue; + } + addrs = rtm->rtm_addrs; + q = (char *)(rtm + 1); if (rtm->rtm_pid == pid) { #if 0 trace(1, "rtmsg looped back to me, ignored\n"); @@ -2973,7 +2982,8 @@ getroute(struct netinfo6 *np, struct in6 exit(1); } rtm = (struct rt_msghdr *)(void *)buf; - } while (rtm->rtm_seq != myseq || rtm->rtm_pid != pid); + } while (rtm->rtm_type != RTM_GET || rtm->rtm_seq != myseq || + rtm->rtm_pid != pid); sin6 = (struct sockaddr_in6 *)(void *)&buf[sizeof(struct rt_msghdr)]; if (rtm->rtm_addrs & RTA_DST) { sin6 = (struct sockaddr_in6 *)(void *) _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
