svn commit: r317035 - in head: contrib/traceroute sbin/route sbin/routed sys/net usr.bin/netstat usr.sbin/arp usr.sbin/ndp usr.sbin/rarpd usr.sbin/route6d
Patrick Kelsey
pkelsey at FreeBSD.org
Sun Apr 16 19:17:12 UTC 2017
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 *)
More information about the svn-src-head
mailing list