svn commit: r328621 - stable/10/sys/netipsec

[Andrey V. Elsukov]

Author: ae
Date: Wed Jan 31 09:26:28 2018
New Revision: 328621
URL: https://svnweb.freebsd.org/changeset/base/328621

Log:
  MFC r328350:
    Merge revision 1.35 from NetBSD:
      fix pointer/offset mistakes in handling of IPv4 options
  
    Reported by:  Maxime Villard <maxv at NetBSD.org>
  
  MFC r328352:
    Adopt revision 1.76 and 1.77 from NetBSD:
      Fix a vulnerability in IPsec-IPv6-AH, that allows an attacker to remotely
      crash the kernel with a single packet.
  
      In this loop we need to increment 'ad' by two, because the length field
      of the option header does not count the size of the option header itself.
  
      If the length is zero, then 'count' is incremented by zero, and there's
      an infinite loop. Beyond that, this code was written with the assumption
      that since the IPv6 packet already went through the generic IPv6 option
      parser, several fields are guaranteed to be valid; but this assumption
      does not hold because of the missing '+2', and there's as a result a
      triggerable buffer overflow (write zeros after the end of the mbuf,
      potentially to the next mbuf in memory since it's a pool).
  
      Add the missing '+2', this place will be reinforced in separate commits.
  
    Reported by:  Maxime Villard <maxv at NetBSD.org>

Modified:
  stable/10/sys/netipsec/xform_ah.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/netipsec/xform_ah.c
==============================================================================
--- stable/10/sys/netipsec/xform_ah.c	Wed Jan 31 09:24:48 2018	(r328620)
+++ stable/10/sys/netipsec/xform_ah.c	Wed Jan 31 09:26:28 2018	(r328621)
@@ -285,7 +285,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 #ifdef INET6
 	struct ip6_ext *ip6e;
 	struct ip6_hdr ip6;
-	int alloc, len, ad;
+	int ad, alloc, nxt, noff;
 #endif /* INET6 */
 
 	switch (proto) {
@@ -314,7 +314,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 		else
 			ip->ip_off = htons(0);
 
-		ptr = mtod(m, unsigned char *) + sizeof(struct ip);
+		ptr = mtod(m, unsigned char *);
 
 		/* IPv4 option processing */
 		for (off = sizeof(struct ip); off < skip;) {
@@ -395,7 +395,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 
 				/* Zeroize all other options. */
 				count = ptr[off + 1];
-				bcopy(ipseczeroes, ptr, count);
+				bcopy(ipseczeroes, ptr + off, count);
 				off += count;
 				break;
 			}
@@ -468,61 +468,44 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 		} else
 			break;
 
-		off = ip6.ip6_nxt & 0xff; /* Next header type. */
+		nxt = ip6.ip6_nxt & 0xff; /* Next header type. */
 
-		for (len = 0; len < skip - sizeof(struct ip6_hdr);)
-			switch (off) {
+		for (off = 0; off < skip - sizeof(struct ip6_hdr);)
+			switch (nxt) {
 			case IPPROTO_HOPOPTS:
 			case IPPROTO_DSTOPTS:
-				ip6e = (struct ip6_ext *) (ptr + len);
+				ip6e = (struct ip6_ext *)(ptr + off);
+				noff = off + ((ip6e->ip6e_len + 1) << 3);
 
+				/* Sanity check. */
+				if (noff > skip - sizeof(struct ip6_hdr))
+					goto error6;
+
 				/*
-				 * Process the mutable/immutable
-				 * options -- borrows heavily from the
-				 * KAME code.
+				 * Zero out mutable options.
 				 */
-				for (count = len + sizeof(struct ip6_ext);
-				     count < len + ((ip6e->ip6e_len + 1) << 3);) {
+				for (count = off + sizeof(struct ip6_ext);
+				     count < noff;) {
 					if (ptr[count] == IP6OPT_PAD1) {
 						count++;
 						continue; /* Skip padding. */
 					}
 
-					/* Sanity check. */
-					if (count > len +
-					    ((ip6e->ip6e_len + 1) << 3)) {
-						m_freem(m);
+					ad = ptr[count + 1] + 2;
+					if (count + ad > noff)
+						goto error6;
 
-						/* Free, if we allocated. */
-						if (alloc)
-							free(ptr, M_XDATA);
-						return EINVAL;
-					}
-
-					ad = ptr[count + 1];
-
-					/* If mutable option, zeroize. */
 					if (ptr[count] & IP6OPT_MUTABLE)
-						bcopy(ipseczeroes, ptr + count,
-						    ptr[count + 1]);
-
+						memset(ptr + count, 0, ad);
 					count += ad;
-
-					/* Sanity check. */
-					if (count >
-					    skip - sizeof(struct ip6_hdr)) {
-						m_freem(m);
-
-						/* Free, if we allocated. */
-						if (alloc)
-							free(ptr, M_XDATA);
-						return EINVAL;
-					}
 				}
 
+				if (count != noff)
+					goto error6;
+
 				/* Advance. */
-				len += ((ip6e->ip6e_len + 1) << 3);
-				off = ip6e->ip6e_nxt;
+				off += ((ip6e->ip6e_len + 1) << 3);
+				nxt = ip6e->ip6e_nxt;
 				break;
 
 			case IPPROTO_ROUTING:
@@ -530,14 +513,15 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 				 * Always include routing headers in
 				 * computation.
 				 */
-				ip6e = (struct ip6_ext *) (ptr + len);
-				len += ((ip6e->ip6e_len + 1) << 3);
-				off = ip6e->ip6e_nxt;
+				ip6e = (struct ip6_ext *) (ptr + off);
+				off += ((ip6e->ip6e_len + 1) << 3);
+				nxt = ip6e->ip6e_nxt;
 				break;
 
 			default:
 				DPRINTF(("%s: unexpected IPv6 header type %d",
 					__func__, off));
+error6:
 				if (alloc)
 					free(ptr, M_XDATA);
 				m_freem(m);