22930543 problem in SERVICE/QUAGGA
authorBrian Utterback <brian.utterback@oracle.com>
Mon, 11 Apr 2016 09:49:37 -0700
changeset 5755 041717cfc591
parent 5754 6ededfcf5619
child 5756 8233953c0160
22930543 problem in SERVICE/QUAGGA
components/quagga/patches/35-cve-2016-2342.patch
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/quagga/patches/35-cve-2016-2342.patch	Mon Apr 11 09:49:37 2016 -0700
@@ -0,0 +1,136 @@
+The fix for this CVE is included in Quagga version 1.0 and later. This patch
+may be removed when Quagga is upgraded to a version later than that. The patch
+is the same as that submitted and subsequently committed to the code base,
+except there is a one line modification to patch. In July 2015 the call to
+decode_label was removed from the code base because it was useless. The
+patch was changed to remove this line. The existence of the line prevented
+the patch from applying as is. After the application of this patch the result
+is the same as is currently available in the community code base.
+
+From: Donald Sharp <[email protected]>
+Date: Wed, 27 Jan 2016 16:54:45 +0000
+Subject: bgpd: Fix VU#270232, VPNv4 NLRI parser memcpys to stack on unchecked length
+
+Address CERT vulnerability report VU#270232, memcpy to stack data structure
+based on length field from packet data whose length field upper-bound was
+not properly checked.
+
+This likely allows BGP peers that are enabled to send Labeled-VPN SAFI
+routes to Quagga bgpd to remotely exploit Quagga bgpd.
+
+Mitigation: Do not enable Labeled-VPN SAFI with untrusted neighbours.
+
+Impact: Labeled-VPN SAFI is not enabled by default.
+
+* bgp_mplsvpn.c: (bgp_nlri_parse_vpnv4) The prefixlen is checked for
+  lower-bound, but not for upper-bound against received data length.
+  The packet data is then memcpy'd to the stack based on the prefixlen.
+
+  Extend the prefixlen check to ensure it is within the bound of the NLRI
+  packet data AND the on-stack prefix structure AND the maximum size for the
+  address family.
+
+Reported-by: Kostya Kortchinsky <[email protected]>
+
+This commit a joint effort between:
+
+Lou Berger <[email protected]>
+Donald Sharp <[email protected]>
+Paul Jakma <[email protected]> / <[email protected]>
+---
+diff --git bgpd/bgp_mplsvpn.c bgpd/bgp_mplsvpn.c
+index a72d5ed..75c90cd 100644
+--- bgpd/bgp_mplsvpn.c
++++ bgpd/bgp_mplsvpn.c
+@@ -102,6 +102,7 @@ bgp_nlri_parse_vpnv4 (struct peer *peer,
+   pnt = packet->nlri;
+   lim = pnt + packet->length;
+ 
++#define VPN_PREFIXLEN_MIN_BYTES (3 + 8) /* label + RD */
+   for (; pnt < lim; pnt += psize)
+     {
+       /* Clear prefix structure. */
+@@ -109,19 +110,38 @@ bgp_nlri_parse_vpnv4 (struct peer *peer,
+ 
+       /* Fetch prefix length. */
+       prefixlen = *pnt++;
+-      p.family = AF_INET;
++      p.family = afi2family (packet->afi);
+       psize = PSIZE (prefixlen);
+ 
+-      if (prefixlen < 88)
+-	{
+-	  zlog_err ("prefix length is less than 88: %d", prefixlen);
+-	  return -1;
+-	}
++      /* sanity check against packet data */
++      if (prefixlen < VPN_PREFIXLEN_MIN_BYTES*8 || (pnt + psize) > lim)
++        {
++          zlog_err ("prefix length (%d) is less than 88"
++                    " or larger than received (%u)",
++                    prefixlen, (uint)(lim-pnt));
++          return -1;
++        }
++
++      /* sanity check against storage for the IP address portion */
++      if ((psize - VPN_PREFIXLEN_MIN_BYTES) > (ssize_t) sizeof(p.u))
++        {
++          zlog_err ("prefix length (%d) exceeds prefix storage (%zu)",
++                    prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, sizeof(p.u));
++          return -1;
++        }
++
++      /* Sanity check against max bitlen of the address family */
++      if ((psize - VPN_PREFIXLEN_MIN_BYTES) > prefix_blen (&p))
++        {
++          zlog_err ("prefix length (%d) exceeds family (%u) max byte length (%u)",
++                    prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, 
++                    p.family, prefix_blen (&p));
++          return -1;
+ 
+-      label = decode_label (pnt);
++        }
+ 
+       /* Copyr label to prefix. */
+-      tagpnt = pnt;;
++      tagpnt = pnt;
+ 
+       /* Copy routing distinguisher to rd. */
+       memcpy (&prd.val, pnt + 3, 8);
+@@ -140,8 +160,9 @@ bgp_nlri_parse_vpnv4 (struct peer *peer,
+ 	  return -1;
+ 	}
+ 
+-      p.prefixlen = prefixlen - 88;
+-      memcpy (&p.u.prefix, pnt + 11, psize - 11);
++      p.prefixlen = prefixlen - VPN_PREFIXLEN_MIN_BYTES*8;
++      memcpy (&p.u.prefix, pnt + VPN_PREFIXLEN_MIN_BYTES, 
++              psize - VPN_PREFIXLEN_MIN_BYTES);
+ 
+ #if 0
+       if (type == RD_TYPE_AS)
+@@ -152,9 +173,6 @@ bgp_nlri_parse_vpnv4 (struct peer *peer,
+ 		   rd_ip.val, inet_ntoa (p.u.prefix4), p.prefixlen);
+ #endif /* 0 */
+ 
+-      if (pnt + psize > lim)
+-	return -1;
+-
+       if (attr)
+ 	bgp_update (peer, &p, attr, AFI_IP, SAFI_MPLS_VPN,
+ 		    ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0);
+@@ -162,12 +180,12 @@ bgp_nlri_parse_vpnv4 (struct peer *peer,
+ 	bgp_withdraw (peer, &p, attr, AFI_IP, SAFI_MPLS_VPN,
+ 		      ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt);
+     }
+-
+   /* Packet length consistency check. */
+   if (pnt != lim)
+     return -1;
+-
++  
+   return 0;
++#undef VPN_PREFIXLEN_MIN_BYTES
+ }
+ 
+ int