Date: Mon, 16 Apr 2007 13:16:09 +0300 From: Mike Makonnen <mtm@FreeBSD.Org> To: freebsd-net@freebsd.org Subject: ping6 extension headers bounds checking Message-ID: <20070416101609.GA2137@rogue.navcom.lan>
next in thread | raw e-mail | index | archive | help
--n8g4imXOkfNTN/H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hello folks, Please review the attached patch for ping6(8) to fix PR kern/99425 You can attach extra headers to the ping6 packet by specifying, for example, extra routing information. This information is sent as control data with sendmsg(2) and when you get a reply is received as control data from recvmsg(2). In a nutshell, there are 2 problems: 1. The buffer supplied to recvmsg(2) to hold control (ancillary) data is, in some cases, too small to hold all the extra headers. 2. In verbose mode, when printing out the control data, it doesn't check to make sure that the stated length of the headers is within the bounds of the buffer. To address this I increased the buffer supplied to recvmsg(2) to the minimum required by rfc 3542 (10420 bytes) and I modified the functions that print the extra header information to print a warning if the buffer is too small and to print only as much information as contained in the buffer. Cheers. -- Mike Makonnen | GPG-KEY: http://people.freebsd.org/~mtm/mtm.asc mmakonnen @ gmail.com | AC7B 5672 2D11 F4D0 EBF8 5279 5359 2B82 7CD4 1F55 mtm @ FreeBSD.Org | FreeBSD - http://www.freebsd.org --n8g4imXOkfNTN/H1 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="ping6.diff" Index: sbin/ping6/ping6.c =================================================================== RCS file: /home/ncvs/src/sbin/ping6/ping6.c,v retrieving revision 1.29 diff -u -u -r1.29 ping6.c --- sbin/ping6/ping6.c 10 Feb 2005 09:19:32 -0000 1.29 +++ sbin/ping6/ping6.c 5 Apr 2007 22:32:52 -0000 @@ -150,6 +150,7 @@ #define ICMP6ECHOLEN 8 /* icmp echo header len excluding time */ #define ICMP6ECHOTMLEN sizeof(struct tv32) #define ICMP6_NIQLEN (ICMP6ECHOLEN + 8) +# define CONTROLLEN 10240 /* ancillary data buffer size RFC3542 20.1 */ /* FQDN case, 64 bits of nonce + 32 bits ttl */ #define ICMP6_NIRLEN (ICMP6ECHOLEN + 12) #define EXTRA 256 /* for AH and various other headers. weird. */ @@ -269,8 +270,8 @@ char *, size_t); void pr_pack(u_char *, int, struct msghdr *); void pr_exthdrs(struct msghdr *); -void pr_ip6opt(void *); -void pr_rthdr(void *); +void pr_ip6opt(void *, unsigned int); +void pr_rthdr(void *, unsigned int); int pr_bitrange(u_int32_t, int, int); void pr_retip(struct ip6_hdr *, u_char *); void summary(void); @@ -307,6 +308,7 @@ char *e, *target, *ifname = NULL, *gateway = NULL; int ip6optlen = 0; struct cmsghdr *scmsgp = NULL; + struct cmsghdr *cm; #if defined(SO_SNDBUF) && defined(SO_RCVBUF) u_long lsockbufsize; int sockbufsize = 0; @@ -1057,10 +1059,13 @@ seeninfo = 0; #endif + /* For control (ancillary) data received from recvmsg() */ + cm = (struct cmsghdr *)malloc(CONTROLLEN); + if (cm == NULL) + err(1, "malloc"); + for (;;) { struct msghdr m; - struct cmsghdr *cm; - u_char buf[1024]; struct iovec iov[2]; /* signal handling */ @@ -1127,9 +1132,9 @@ iov[0].iov_len = packlen; m.msg_iov = iov; m.msg_iovlen = 1; - cm = (struct cmsghdr *)buf; - m.msg_control = (caddr_t)buf; - m.msg_controllen = sizeof(buf); + memset(cm, 0, CONTROLLEN); + m.msg_control = (void *)cm; + m.msg_controllen = CONTROLLEN; cc = recvmsg(s, &m, 0); if (cc < 0) { @@ -1493,6 +1498,9 @@ pr_addr(from, fromlen)); return; } + if (((mhdr->msg_flags & MSG_CTRUNC) != 0) && + (options & F_VERBOSE) != 0) + warnx("some control data discarded, insufficient buffer size"); icp = (struct icmp6_hdr *)buf; ni = (struct icmp6_nodeinfo *)buf; off = 0; @@ -1735,28 +1743,31 @@ pr_exthdrs(mhdr) struct msghdr *mhdr; { - struct cmsghdr *cm; + struct cmsghdr *cm, *cm1; + unsigned int offset; - for (cm = (struct cmsghdr *)CMSG_FIRSTHDR(mhdr); cm; - cm = (struct cmsghdr *)CMSG_NXTHDR(mhdr, cm)) { + offset = 0; + cm1 = (struct cmsghdr *)CMSG_FIRSTHDR(mhdr); + for (cm = cm1; cm; cm = (struct cmsghdr *)CMSG_NXTHDR(mhdr, cm)) { if (cm->cmsg_level != IPPROTO_IPV6) continue; + offset = (caddr_t)CMSG_DATA(cm) - (caddr_t)cm1; switch (cm->cmsg_type) { case IPV6_HOPOPTS: printf(" HbH Options: "); - pr_ip6opt(CMSG_DATA(cm)); + pr_ip6opt(CMSG_DATA(cm), offset); break; case IPV6_DSTOPTS: #ifdef IPV6_RTHDRDSTOPTS case IPV6_RTHDRDSTOPTS: #endif printf(" Dst Options: "); - pr_ip6opt(CMSG_DATA(cm)); + pr_ip6opt(CMSG_DATA(cm), offset); break; case IPV6_RTHDR: printf(" Routing: "); - pr_rthdr(CMSG_DATA(cm)); + pr_rthdr(CMSG_DATA(cm), offset); break; } } @@ -1764,12 +1775,12 @@ #ifdef USE_RFC2292BIS void -pr_ip6opt(void *extbuf) +pr_ip6opt(void *extbuf, unsigned int cmoffset) { struct ip6_hbh *ext; int currentlen; u_int8_t type; - socklen_t extlen, len; + socklen_t extlen, len, origextlen; void *databuf; size_t offset; u_int16_t value2; @@ -1780,6 +1791,21 @@ printf("nxt %u, len %u (%lu bytes)\n", ext->ip6h_nxt, (unsigned int)ext->ip6h_len, (unsigned long)extlen); + /* + * Bounds checking on the ancillary data buffer. When calculating + * the number of items to show keep in mind: + * - The offset, in the ancillary data buffer, of this header + * - The size of the cmsg structure + * - The size of one unit (8 octets) + */ + if (CONTROLLEN < (extlen + CMSG_SPACE(0) + offset)) { + origextlen = extlen; + extlen -= (extlen - (CONTROLLEN - offset - CMSG_SPACE(0))) / 8; + warnx("options truncated, showing only %u (total=%u)", + (unsigned int)(extlen / 8 - 1), + (unsigned int)(ext->ip6h_len)); + } + currentlen = 0; while (1) { currentlen = inet6_opt_next(extbuf, extlen, currentlen, @@ -1816,7 +1842,7 @@ #else /* !USE_RFC2292BIS */ /* ARGSUSED */ void -pr_ip6opt(void *extbuf) +pr_ip6opt(void *extbuf, unsigned int cmoffset __unused) { putchar('\n'); return; @@ -1825,21 +1851,44 @@ #ifdef USE_RFC2292BIS void -pr_rthdr(void *extbuf) +pr_rthdr(void *extbuf, unsigned int offset) { struct in6_addr *in6; char ntopbuf[INET6_ADDRSTRLEN]; struct ip6_rthdr *rh = (struct ip6_rthdr *)extbuf; - int i, segments; + int i, segments, origsegs, rthsize, size0, size1; /* print fixed part of the header */ printf("nxt %u, len %u (%d bytes), type %u, ", rh->ip6r_nxt, rh->ip6r_len, (rh->ip6r_len + 1) << 3, rh->ip6r_type); - if ((segments = inet6_rth_segments(extbuf)) >= 0) + if ((segments = inet6_rth_segments(extbuf)) >= 0) { printf("%d segments, ", segments); - else + printf("%d left\n", rh->ip6r_segleft); + } else { printf("segments unknown, "); - printf("%d left\n", rh->ip6r_segleft); + printf("%d left\n", rh->ip6r_segleft); + return; + } + + /* + * Bounds checking on the ancillary data buffer. When calculating + * the number of items to show keep in mind: + * - The offset, in the ancillary data buffer, of this header + * - The size of the cmsg structure + * - The size of one segment (the size of one IPv6 address) + * - When dividing add a fudge factor of one in case the + * dividend is not evenly divisible by the divisor + */ + rthsize = (rh->ip6r_len + 1) * 8; + if (CONTROLLEN < (rthsize + CMSG_SPACE(0) + offset)) { + origsegs = segments; + size0 = inet6_rth_space(IPV6_RTHDR_TYPE_0, 0); + size1 = inet6_rth_space(IPV6_RTHDR_TYPE_0, 1); + segments -= (rthsize - (CONTROLLEN - offset - CMSG_SPACE(0))) / + (size1 - size0) + 1; + warnx("segments truncated, showing only %d (total=%d)", + segments, origsegs); + } for (i = 0; i < segments; i++) { in6 = inet6_rth_getaddr(extbuf, i); @@ -1860,7 +1909,7 @@ #else /* !USE_RFC2292BIS */ /* ARGSUSED */ void -pr_rthdr(void *extbuf) +pr_rthdr(void *extbuf, unsigned int offset __unused) { putchar('\n'); return; --n8g4imXOkfNTN/H1--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070416101609.GA2137>