1 files changed, 79 insertions, 81 deletions

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index cf930427..776c7127 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -91,13 +91,13 @@ static struct hash *ashash;
 /* Stream for SNMP. See aspath_snmp_pathseg */
 static struct stream *snmp_stream;
 
-static inline as_t *
+static as_t *
 assegment_data_new (int num)
 {
   return (XCALLOC (MTYPE_AS_SEG_DATA, ASSEGMENT_DATA_SIZE (num, 1)));
 }
 
-static inline void
+static void
 assegment_data_free (as_t *asdata)
 {
   XFREE (MTYPE_AS_SEG_DATA,asdata);
@@ -340,19 +340,21 @@ aspath_free (struct aspath *aspath)
 
 /* Unintern aspath from AS path bucket. */
 void
-aspath_unintern (struct aspath *aspath)
+aspath_unintern (struct aspath **aspath)
 {
   struct aspath *ret;
+  struct aspath *asp = *aspath;
+  
+  if (asp->refcnt)
+    asp->refcnt--;
 
-  if (aspath->refcnt)
-    aspath->refcnt--;
-
-  if (aspath->refcnt == 0)
+  if (asp->refcnt == 0)
     {
       /* This aspath must exist in aspath hash table. */
-      ret = hash_release (ashash, aspath);
+      ret = hash_release (ashash, asp);
       assert (ret != NULL);
-      aspath_free (aspath);
+      aspath_free (asp);
+      *aspath = NULL;
     }
 }
 
@@ -671,80 +673,79 @@ aspath_hash_alloc (void *arg)
   return aspath;
 }
 
-/* parse as-segment byte stream in struct assegment
- *
- * Returns NULL if the AS_PATH or AS4_PATH is not valid.
- */
-static struct assegment *
-assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+/* parse as-segment byte stream in struct assegment */
+static int
+assegments_parse (struct stream *s, size_t length, 
+                  struct assegment **result, int use32bit)
 {
   struct assegment_header segh;
   struct assegment *seg, *prev = NULL, *head = NULL;
+  size_t bytes = 0;
   
-  assert (length > 0);  /* does not expect empty AS_PATH or AS4_PATH    */
+  /* empty aspath (ie iBGP or somesuch) */
+  if (length == 0)
+    return 0;
   
   if (BGP_DEBUG (as4, AS4_SEGMENT))
     zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu",
 		(unsigned long) length);
-
-  /* double check that length does not exceed stream    */
-  if (STREAM_READABLE(s) < length)
-    return NULL;
+  /* basic checks */
+  if ((STREAM_READABLE(s) < length)
+      || (STREAM_READABLE(s) < AS_HEADER_SIZE) 
+      || (length % AS16_VALUE_SIZE ))
+    return -1;
   
-  /* deal with each segment in turn                             */
-  while (length > 0)
+  while (bytes < length)
     {
       int i;
       size_t seg_size;
       
-      /* softly softly, get the header first on its own */
-      if (length < AS_HEADER_SIZE)
+      if ((length - bytes) <= AS_HEADER_SIZE)
         {
-          assegment_free_all (head);
-          return NULL;
+          if (head)
+            assegment_free_all (head);
+          return -1;
         }
       
+      /* softly softly, get the header first on its own */
       segh.type = stream_getc (s);
       segh.length = stream_getc (s);
       
       seg_size = ASSEGMENT_SIZE(segh.length, use32bit);
-                                      /* includes the header bytes */
 
       if (BGP_DEBUG (as4, AS4_SEGMENT))
 	zlog_debug ("[AS4SEG] Parse aspath segment: got type %d, length %d",
                     segh.type, segh.length);
       
+      /* check it.. */
+      if ( ((bytes + seg_size) > length)
+          /* 1771bis 4.3b: seg length contains one or more */
+          || (segh.length == 0) 
+          /* Paranoia in case someone changes type of segment length.
+           * Shift both values by 0x10 to make the comparison operate
+           * on more, than 8 bits (otherwise it's a warning, bug #564).
+           */
+          || ((sizeof segh.length > 1) 
+              && (0x10 + segh.length > 0x10 + AS_SEGMENT_MAX)))
+        {
+          if (head)
+            assegment_free_all (head);
+          return -1;
+        }
+      
       switch (segh.type)
         {
           case AS_SEQUENCE:
           case AS_SET:
-            break ;
-
           case AS_CONFED_SEQUENCE:
           case AS_CONFED_SET:
-            if (!as4_path)
-              break ;
-              /* RFC4893 3: "invalid for the AS4_PATH attribute"            */
-              /* fall through */
-
-          default:    /* reject unknown or invalid AS_PATH segment types  */
-            seg_size = 0 ;
-        }
-     
-     /* Stop now if segment is not valid (discarding anything collected to date)
-      *
-      * RFC4271 4.3, Path Attributes, b) AS_PATH:
-      *
-      *   "path segment value field contains one or more AS numbers"
-           */
-      if ((seg_size == 0) || (seg_size > length) || (segh.length == 0))
-        {
-          assegment_free_all (head);
-          return NULL;
+            break;
+          default:
+            if (head)
+              assegment_free_all (head);
+            return -1;
         }
       
-      length -= seg_size ;
-      
       /* now its safe to trust lengths */
       seg = assegment_new (segh.type, segh.length);
       
@@ -756,52 +757,47 @@ assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path)
       for (i = 0; i < segh.length; i++)
 	seg->as[i] = (use32bit) ? stream_getl (s) : stream_getw (s);
 
+      bytes += seg_size;
+      
       if (BGP_DEBUG (as4, AS4_SEGMENT))
-	zlog_debug ("[AS4SEG] Parse aspath segment: length left: %lu",
-	            (unsigned long) length);
+	zlog_debug ("[AS4SEG] Parse aspath segment: Bytes now: %lu",
+	            (unsigned long) bytes);
       
       prev = seg;
     }
  
-  return assegment_normalise (head);
+  *result = assegment_normalise (head);
+  return 0;
 }
 
-/* AS path parse function -- parses AS_PATH and AS4_PATH attributes
- *
- * Requires: s        -- stream, currently positioned before first segment
- *                       of AS_PATH or AS4_PATH (ie after attribute header)
- *           length   -- length of the value of the AS_PATH or AS4_PATH
- *           use32bit -- true <=> 4Byte ASN, otherwise 2Byte ASN
- *           as4_path -- true <=> AS4_PATH, otherwise AS_PATH
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- *                    with reference count incremented.
- *              else: NULL
- *
- * NB: empty AS path (length == 0) is valid.  The returned struct aspath will
- *     have segments == NULL and str == zero length string (unique).
+/* AS path parse function.  pnt is a pointer to byte stream and length
+   is length of byte stream.  If there is same AS path in the the AS
+   path hash then return it else make new AS path structure. 
+   
+   On error NULL is returned.
  */
 struct aspath *
-aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+aspath_parse (struct stream *s, size_t length, int use32bit)
 {
   struct aspath as;
   struct aspath *find;
 
-  /* Parse each segment and construct normalised list of struct assegment */
-  memset (&as, 0, sizeof (struct aspath));
-  if (length != 0)
-    {
-      as.segments = assegments_parse (s, length, use32bit, as4_path);
+  /* If length is odd it's malformed AS path. */
+  /* Nit-picking: if (use32bit == 0) it is malformed if odd,
+   * otherwise its malformed when length is larger than 2 and (length-2) 
+   * is not dividable by 4.
+   * But... this time we're lazy
+   */
+  if (length % AS16_VALUE_SIZE )
+    return NULL;
 
-      if (as.segments == NULL)
-        return NULL ;   /* Invalid AS_PATH or AS4_PATH  */
-    } ;
+  memset (&as, 0, sizeof (struct aspath));
+  if (assegments_parse (s, length, &as.segments, use32bit) < 0)
+    return NULL;
   
   /* If already same aspath exist then return it. */
   find = hash_get (ashash, &as, aspath_hash_alloc);
   
-  assert(find) ;        /* valid aspath, so must find or create */
-  
   /* aspath_hash_alloc dupes segments too. that probably could be
    * optimised out.
    */
@@ -809,12 +805,14 @@ aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path)
   if (as.str)
     XFREE (MTYPE_AS_STR, as.str);
   
+  if (! find)
+    return NULL;
   find->refcnt++;
 
   return find;
 }
 
-static inline void
+static void
 assegment_data_put (struct stream *s, as_t *as, int num, int use32bit)
 {
   int i;
@@ -832,7 +830,7 @@ assegment_data_put (struct stream *s, as_t *as, int num, int use32bit)
       }
 }
 
-static inline size_t
+static size_t
 assegment_header_put (struct stream *s, u_char type, int length)
 {
   size_t lenp;
@@ -1632,7 +1630,7 @@ aspath_segment_add (struct aspath *as, int type)
 struct aspath *
 aspath_empty (void)
 {
-  return aspath_parse (NULL, 0, 1, 0); /* 32Bit ;-) not AS4_PATH */
+  return aspath_parse (NULL, 0, 1); /* 32Bit ;-) */
 }
 
 struct aspath *