c26cda4
From 7819e36e64189f2073eff16dd9359b8192605a7b Mon Sep 17 00:00:00 2001
c26cda4
From: Donald Sharp <sharpd@cumulusnetworks.com>
c26cda4
Date: Wed, 27 Jan 2016 16:54:45 +0000
c26cda4
Subject: [PATCH] bgpd: Fix VU#270232, VPNv4 NLRI parser memcpys to stack on
c26cda4
 unchecked length
c26cda4
c26cda4
Address CERT vulnerability report VU#270232, memcpy to stack data structure
c26cda4
based on length field from packet data whose length field upper-bound was
c26cda4
not properly checked.
c26cda4
c26cda4
This likely allows BGP peers that are enabled to send Labeled-VPN SAFI
c26cda4
routes to Quagga bgpd to remotely exploit Quagga bgpd.
c26cda4
c26cda4
Mitigation: Do not enable Labeled-VPN SAFI with untrusted neighbours.
c26cda4
c26cda4
Impact: Labeled-VPN SAFI is not enabled by default.
c26cda4
c26cda4
* bgp_mplsvpn.c: (bgp_nlri_parse_vpnv4) The prefixlen is checked for
c26cda4
  lower-bound, but not for upper-bound against received data length.
c26cda4
  The packet data is then memcpy'd to the stack based on the prefixlen.
c26cda4
c26cda4
  Extend the prefixlen check to ensure it is within the bound of the NLRI
c26cda4
  packet data AND the on-stack prefix structure AND the maximum size for the
c26cda4
  address family.
c26cda4
c26cda4
Reported-by: Kostya Kortchinsky <kostyak@google.com>
c26cda4
c26cda4
This commit a joint effort between:
c26cda4
c26cda4
Lou Berger <lberger@labn.net>
c26cda4
Donald Sharp <sharpd@cumulusnetworks.com>
c26cda4
Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>
c26cda4
c26cda4
Resolves: #1316572
c26cda4
Cherry-picked from: a3bc7e9400b214a0f078fdb19596ba54214a1442)
c26cda4
Conflicts:
c26cda4
	bgpd/bgp_mplsvpn.c
c26cda4
---
c26cda4
 bgpd/bgp_mplsvpn.c | 55 +++++++++++++++++++++++++++++++++++-------------------
c26cda4
 1 file changed, 36 insertions(+), 19 deletions(-)
c26cda4
c26cda4
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
c26cda4
index 3d2dade..42eba55 100644
c26cda4
--- a/bgpd/bgp_mplsvpn.c
c26cda4
+++ b/bgpd/bgp_mplsvpn.c
c26cda4
@@ -101,6 +101,7 @@ bgp_nlri_parse_vpnv4 (struct peer *peer, struct attr *attr,
c26cda4
   pnt = packet->nlri;
c26cda4
   lim = pnt + packet->length;
c26cda4
 
c26cda4
+#define VPN_PREFIXLEN_MIN_BYTES (3 + 8) /* label + RD */
c26cda4
   for (; pnt < lim; pnt += psize)
c26cda4
     {
c26cda4
       /* Clear prefix structure. */
c26cda4
@@ -108,20 +109,38 @@ bgp_nlri_parse_vpnv4 (struct peer *peer, struct attr *attr,
c26cda4
 
c26cda4
       /* Fetch prefix length. */
c26cda4
       prefixlen = *pnt++;
c26cda4
-      p.family = AF_INET;
c26cda4
+      p.family = afi2family (packet->afi);
c26cda4
       psize = PSIZE (prefixlen);
c26cda4
-
c26cda4
-      if (prefixlen < 88)
c26cda4
-	{
c26cda4
-	  zlog_err ("prefix length is less than 88: %d", prefixlen);
c26cda4
-	  return -1;
c26cda4
-	}
c26cda4
-	
c26cda4
-      /* XXX: Not doing anything with the label */
c26cda4
-      decode_label (pnt);
c26cda4
-
c26cda4
+      
c26cda4
+      /* sanity check against packet data */
c26cda4
+      if (prefixlen < VPN_PREFIXLEN_MIN_BYTES*8 || (pnt + psize) > lim)
c26cda4
+        {
c26cda4
+          zlog_err ("prefix length (%d) is less than 88"
c26cda4
+                    " or larger than received (%u)",
c26cda4
+                    prefixlen, (uint)(lim-pnt));
c26cda4
+          return -1;
c26cda4
+        }
c26cda4
+      
c26cda4
+      /* sanity check against storage for the IP address portion */
c26cda4
+      if ((psize - VPN_PREFIXLEN_MIN_BYTES) > (ssize_t) sizeof(p.u))
c26cda4
+        {
c26cda4
+          zlog_err ("prefix length (%d) exceeds prefix storage (%zu)",
c26cda4
+                    prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, sizeof(p.u));
c26cda4
+          return -1;
c26cda4
+        }
c26cda4
+      
c26cda4
+      /* Sanity check against max bitlen of the address family */
c26cda4
+      if ((psize - VPN_PREFIXLEN_MIN_BYTES) > prefix_blen (&p))
c26cda4
+        {
c26cda4
+          zlog_err ("prefix length (%d) exceeds family (%u) max byte length (%u)",
c26cda4
+                    prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, 
c26cda4
+                    p.family, prefix_blen (&p);;
c26cda4
+          return -1;
c26cda4
+                  
c26cda4
+        }
c26cda4
+      
c26cda4
       /* Copyr label to prefix. */
c26cda4
-      tagpnt = pnt;;
c26cda4
+      tagpnt = pnt;
c26cda4
 
c26cda4
       /* Copy routing distinguisher to rd. */
c26cda4
       memcpy (&prd.val, pnt + 3, 8);
c26cda4
@@ -140,8 +159,9 @@ bgp_nlri_parse_vpnv4 (struct peer *peer, struct attr *attr,
c26cda4
 	  return -1;
c26cda4
 	}
c26cda4
 
c26cda4
-      p.prefixlen = prefixlen - 88;
c26cda4
-      memcpy (&p.u.prefix, pnt + 11, psize - 11);
c26cda4
+      p.prefixlen = prefixlen - VPN_PREFIXLEN_MIN_BYTES*8;
c26cda4
+      memcpy (&p.u.prefix, pnt + VPN_PREFIXLEN_MIN_BYTES, 
c26cda4
+              psize - VPN_PREFIXLEN_MIN_BYTES);
c26cda4
 
c26cda4
 #if 0
c26cda4
       if (type == RD_TYPE_AS)
c26cda4
@@ -152,9 +172,6 @@ bgp_nlri_parse_vpnv4 (struct peer *peer, struct attr *attr,
c26cda4
 		   rd_ip.val, inet_ntoa (p.u.prefix4), p.prefixlen);
c26cda4
 #endif /* 0 */
c26cda4
 
c26cda4
-      if (pnt + psize > lim)
c26cda4
-	return -1;
c26cda4
-
c26cda4
       if (attr)
c26cda4
 	bgp_update (peer, &p, attr, AFI_IP, SAFI_MPLS_VPN,
c26cda4
 		    ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0);
c26cda4
@@ -162,12 +179,12 @@ bgp_nlri_parse_vpnv4 (struct peer *peer, struct attr *attr,
c26cda4
 	bgp_withdraw (peer, &p, attr, AFI_IP, SAFI_MPLS_VPN,
c26cda4
 		      ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt);
c26cda4
     }
c26cda4
-
c26cda4
   /* Packet length consistency check. */
c26cda4
   if (pnt != lim)
c26cda4
     return -1;
c26cda4
-
c26cda4
+  
c26cda4
   return 0;
c26cda4
+#undef VPN_PREFIXLEN_MIN_BYTES
c26cda4
 }
c26cda4
 
c26cda4
 int
c26cda4
-- 
c26cda4
2.7.4
c26cda4