bgpd/security: CVE-2010-1674 Fix crash due to extended-community parser error

* bgp_attr.c: (bgp_attr_ext_communities) Certain extended-community attrs
  can leave attr->flag indicating ext-community is present, even though no
  extended-community object has been attached to the attr structure.  Thus a
  null-pointer dereference can occur later.
  (bgp_attr_community) No bug fixed here, but tidy up flow so it has same
  form as previous.

  Problem and fix thanks to anonymous reporter.
(cherry picked from commit 0c46638122f10019a12ae9668aec91691cf2e017)

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index ae0dc88aee619ba0e378436a49b74268506eda6e..c6fd3a573667621fb8aaa399b92b8f504f7569a6 100644 (file)
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1235,13 +1235,16 @@ bgp_attr_community (struct peer *peer, bgp_size_t length,
       attr->community = NULL;
       return 0;
     }
-  else
-    {
-      attr->community = 
-        community_parse ((u_int32_t *)stream_pnt (peer->ibuf), length);
-      stream_forward_getp (peer->ibuf, length);
-    }
+  
+  attr->community =
+    community_parse ((u_int32_t *)stream_pnt (peer->ibuf), length);
+  
+  /* XXX: fix community_parse to use stream API and remove this */
+  stream_forward_getp (peer->ibuf, length);
 
+  if (!attr->community)
+    return -1;
+  
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
 
   return 0;
@@ -1478,13 +1481,18 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
     {
       if (attr->extra)
         attr->extra->ecommunity = NULL;
+      /* Empty extcomm doesn't seem to be invalid per se */
+      return 0;
     }
-  else
-    {
-      (bgp_attr_extra_get (attr))->ecommunity = 
-        ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length);
-      stream_forward_getp (peer->ibuf, length);
-    }
+
+  (bgp_attr_extra_get (attr))->ecommunity =
+    ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length);
+  /* XXX: fix ecommunity_parse to use stream API */
+  stream_forward_getp (peer->ibuf, length);
+  
+  if (!attr->extra->ecommunity)
+    return -1;
+  
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
 
   return 0;