pullup ticket #2376 - requested by tonnerre
quagga: fixes denial of service

revisions pulled up:
- pkgsrc/net/quagga/Makefile		1.31
- pkgsrc/net/quagga/distinfo		1.10
- pkgsrc/net/quagga/patches/patch-ab	1.3
- pkgsrc/net/quagga/patches/patch-ac	1.3

   Module Name:	pkgsrc
   Committed By:	tonnerre
   Date:		Tue May 13 22:30:47 UTC 2008

   Modified Files:
   	pkgsrc/net/quagga: Makefile distinfo
   Added Files:
   	pkgsrc/net/quagga/patches: patch-ab patch-ac

   Log Message:
   Add patch for CVE-2007-1995 for stable quagga (NLRI attributes denial of
   service).


(rtr)

$NetBSD: patch-ab,v 1.2.26.1 2008/05/15 08:44:57 rtr Exp $

--- bgpd/bgp_attr.c
+++ bgpd/bgp_attr.c
@@ -39,7 +39,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "bgpd/bgp_ecommunity.h"
 
 /* Attribute strings for logging. */
-struct message attr_str [] = 
+static struct message attr_str [] = 
 {
   { BGP_ATTR_ORIGIN,           "ORIGIN" }, 
   { BGP_ATTR_AS_PATH,          "AS_PATH" }, 
@@ -58,6 +58,7 @@ struct message attr_str [] =
   { BGP_ATTR_MP_UNREACH_NLRI,  "MP_UNREACH_NLRI" },
   { 0, NULL }
 };
+int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]);
 
 struct hash *cluster_hash;
 
@@ -922,24 +923,30 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
 {
   u_int16_t afi;
   u_char safi;
-  u_char snpa_num;
-  u_char snpa_len;
-  u_char *lim;
   bgp_size_t nlri_len;
+  size_t start;
   int ret;
   struct stream *s;
   
   /* Set end of packet. */
-  s = peer->ibuf;
-  lim = stream_pnt (s) + length;
-
+  s = BGP_INPUT(peer);
+  start = stream_get_getp(s);
+  
+  /* safe to read statically sized header? */
+#define BGP_MP_REACH_MIN_SIZE 5
+  if ((length > STREAM_READABLE(s)) || (length < BGP_MP_REACH_MIN_SIZE))
+    return -1;
+  
   /* Load AFI, SAFI. */
   afi = stream_getw (s);
   safi = stream_getc (s);
 
   /* Get nexthop length. */
   attr->mp_nexthop_len = stream_getc (s);
-
+  
+  if (STREAM_READABLE(s) < attr->mp_nexthop_len)
+    return -1;
+  
   /* Nexthop length check. */
   switch (attr->mp_nexthop_len)
     {
@@ -986,31 +993,28 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
       break;
     }
 
-  snpa_num = stream_getc (s);
-
-  while (snpa_num--)
-    {
-      snpa_len = stream_getc (s);
-      stream_forward (s, (snpa_len + 1) >> 1);
-    }
+  if (!STREAM_READABLE(s))
+    return -1;
+  
+  {
+    u_char val; 
+    if ((val = stream_getc (s)))
+    zlog_warn ("%s sent non-zero value, %u, for defunct SNPA-length field",
+               peer->host, val);
+  }
+  
+  /* must have nrli_len, what is left of the attribute */
+  nlri_len = length - (stream_get_getp(s) - start);
+  if ((!nlri_len) || (nlri_len > STREAM_READABLE(s)))
+    return -1;
   
-  /* If peer is based on old draft-00. I read NLRI length from the
-     packet. */
-  if (peer->version == BGP_VERSION_MP_4_DRAFT_00)
-    {
-      bgp_size_t nlri_total_len;
-      nlri_total_len = stream_getw (s);
-    }
-
-  nlri_len = lim - stream_pnt (s);
- 
   if (safi != BGP_SAFI_VPNV4)
     {
       ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), nlri_len);
       if (ret < 0)
 	return -1;
     }
-
+  
   mp_update->afi = afi;
   mp_update->safi = safi;
   mp_update->nlri = stream_pnt (s);
@@ -1023,24 +1027,26 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
 
 /* Multiprotocol unreachable parse */
 int
-bgp_mp_unreach_parse (struct peer *peer, int length, 
+bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length, 
 		      struct bgp_nlri *mp_withdraw)
 {
   struct stream *s;
   u_int16_t afi;
   u_char safi;
-  u_char *lim;
   u_int16_t withdraw_len;
   int ret;
 
   s = peer->ibuf;
-  lim = stream_pnt (s) + length;
 
+#define BGP_MP_UNREACH_MIN_SIZE 3
+  if ((length > STREAM_READABLE(s)) || (length <  BGP_MP_UNREACH_MIN_SIZE))
+    return -1;
+  
   afi = stream_getw (s);
   safi = stream_getc (s);
-
-  withdraw_len = lim - stream_pnt (s);
-
+  
+  withdraw_len = length - BGP_MP_UNREACH_MIN_SIZE;
+  
   if (safi != BGP_SAFI_VPNV4)
     {
       ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
@@ -1271,13 +1277,23 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 
       /* If error occured immediately return to the caller. */
       if (ret < 0)
-	return ret;
+        {
+          zlog (peer->log, LOG_WARNING,
+                "%s: Attribute %s, parse error", 
+                peer->host, 
+                LOOKUP (attr_str, type));
+           bgp_notify_send (peer, 
+                            BGP_NOTIFY_UPDATE_ERR,
+                            BGP_NOTIFY_UPDATE_MAL_ATTR);
+           return ret;
+        }
 
       /* Check the fetched length. */
       if (BGP_INPUT_PNT (peer) != attr_endp)
 	{
 	  zlog (peer->log, LOG_WARNING, 
-		"%s BGP attribute fetch error", peer->host);
+		"%s: BGP attribute %s, fetch error", 
+                peer->host, LOOKUP (attr_str, type));
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
@@ -1289,7 +1305,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
   if (BGP_INPUT_PNT (peer) != endp)
     {
       zlog (peer->log, LOG_WARNING, 
-	    "%s BGP attribute length mismatch", peer->host);
+	    "%s BGP attribute %s, length mismatch",
+	    peer->host, LOOKUP (attr_str, type));
       bgp_notify_send (peer, 
 		       BGP_NOTIFY_UPDATE_ERR, 
 		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
diff --git a/doc/quagga.info b/doc/quagga.info
diff --git a/lib/stream.h b/lib/stream.h
index f7a94ea..a85e413 100644

$NetBSD: patch-ac,v 1.2.26.1 2008/05/15 08:44:57 rtr Exp $

--- lib/stream.h
+++ lib/stream.h
@@ -59,7 +59,9 @@ struct stream_fifo
 #define STREAM_SIZE(S)  ((S)->size)
 #define STREAM_REMAIN(S) ((S)->size - (S)->putp)
 #define STREAM_DATA(S)  ((S)->data)
-
+/* number of bytes still to be read */
+#define STREAM_READABLE(S) ((S)->endp - (S)->getp)
+  
 /* Stream prototypes. */
 struct stream *stream_new (size_t);
 void stream_free (struct stream *);