bgpd: improve attr length error handling (BZ#679)

* bgp_attr.c
  * bgp_attr_parse(): provide extra argument to bgp_attr_aggregator()
  * bgp_attr_local_pref(): use bgp_notify_send_with_data()
  * bgp_attr_atomic(): idem
  * bgp_attr_aggregator(): idem

Conflicts:

	bgpd/bgp_attr.c

1 files changed, 19 insertions, 12 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index daf14bd4..5c3d153a 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1085,6 +1085,16 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
     bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
     return -1;
   }
+  /* Length check. */
+  if (length != 4)
+  {
+    zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]", length);
+    bgp_notify_send_with_data (peer,
+                               BGP_NOTIFY_UPDATE_ERR,
+                               BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                               startp, total);
+    return -1;
+  }
 
   /* If it is contained in an UPDATE message that is received from an
      external peer, then this attribute MUST be ignored by the
@@ -1095,10 +1105,7 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
       return BGP_ATTR_PARSE_PROCEED;
     }
 
-  if (length == 4) 
-    attr->local_pref = stream_getl (peer->ibuf);
-  else 
-    attr->local_pref = 0;
+  attr->local_pref = stream_getl (peer->ibuf);
 
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);
@@ -1130,11 +1137,10 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length,
   /* Length check. */
   if (length != 0)
     {
-      zlog (peer->log, LOG_ERR, "Bad atomic aggregate length %d", length);
-
+      zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute length isn't 0 [%u]", length);
       return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
                                  BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-                                 NULL, 0);
+                                 startp, total);
     }
 
   /* Set atomic aggregate flag. */
@@ -1146,22 +1152,23 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length,
 /* Aggregator attribute */
 static int
 bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
-		     struct attr *attr, u_char flag)
+		     struct attr *attr, u_char flag, u_char *startp)
 {
   int wantedlen = 6;
   struct attr_extra *attre = bgp_attr_extra_get (attr);
+  bgp_size_t total;
   
+  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
   /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
   if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
     wantedlen = 8;
   
   if (length != wantedlen)
     {
-      zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length);
-
+      zlog (peer->log, LOG_ERR, "AGGREGATOR attribute length isn't %u [%u]", wantedlen, length);
       return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
                                  BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-                                 NULL, 0);
+                                 startp, total);
     }
   
   if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@@ -1789,7 +1796,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_attr_atomic (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AGGREGATOR:
-	  ret = bgp_attr_aggregator (peer, length, attr, flag);
+	  ret = bgp_attr_aggregator (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS4_AGGREGATOR:
 	  ret = bgp_attr_as4_aggregator (peer, length, attr, flag,