ba3a7a2
From cc2e6770697e343f4af534114ab7e633d5beabec Mon Sep 17 00:00:00 2001
ba3a7a2
From: Paul Jakma <paul@jakma.org>
ba3a7a2
Date: Wed, 3 Jan 2018 23:57:33 +0000
ba3a7a2
Subject: [PATCH] bgpd/security: invalid attr length sends NOTIFY with data
ba3a7a2
 overrun
ba3a7a2
ba3a7a2
Security issue: Quagga-2018-0543
ba3a7a2
ba3a7a2
See: https://www.quagga.net/security/Quagga-2018-0543.txt
ba3a7a2
ba3a7a2
* bgpd/bgp_attr.c: (bgp_attr_parse) An invalid attribute length is correctly
ba3a7a2
  checked, and a NOTIFY prepared.  The NOTIFY can include the incorrect
ba3a7a2
  received data with the NOTIFY, for debug purposes.  Commit
ba3a7a2
  c69698704806a9ac5 modified the code to do that just, and also send the
ba3a7a2
  malformed attr with the NOTIFY.  However, the invalid attribute length was
ba3a7a2
  used as the length of the data to send back.
ba3a7a2
ba3a7a2
  The result is a read past the end of data, which is then written to the
ba3a7a2
  NOTIFY message and sent to the peer.
ba3a7a2
ba3a7a2
  A configured BGP peer can use this bug to read up to 64 KiB of memory from
ba3a7a2
  the bgpd process, or crash the process if the invalid read is caught by
ba3a7a2
  some means (unmapped page and SEGV, or other mechanism) resulting in a DoS.
ba3a7a2
ba3a7a2
  This bug _ought_ /not/ be exploitable by anything other than the connected
ba3a7a2
  BGP peer, assuming the underlying TCP transport is secure.  For no BGP
ba3a7a2
  peer should send on an UPDATE with this attribute.  Quagga will not, as
ba3a7a2
  Quagga always validates the attr header length, regardless of type.
ba3a7a2
ba3a7a2
  However, it is possible that there are BGP implementations that do not
ba3a7a2
  check lengths on some attributes (e.g.  optional/transitive ones of a type
ba3a7a2
  they do not recognise), and might pass such malformed attrs on.  If such
ba3a7a2
  implementations exists and are common, then this bug might be triggerable
ba3a7a2
  by BGP speakers further hops away.  Those peers will not receive the
ba3a7a2
  NOTIFY (unless they sit on a shared medium), however they might then be
ba3a7a2
  able to trigger a DoS.
ba3a7a2
ba3a7a2
  Fix: use the valid bound to calculate the length.
ba3a7a2
---
ba3a7a2
 bgpd/bgp_attr.c | 4 +++-
ba3a7a2
 1 file changed, 3 insertions(+), 1 deletion(-)
ba3a7a2
ba3a7a2
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
ba3a7a2
index ef58beb1..9564637e 100644
ba3a7a2
--- a/bgpd/bgp_attr.c
ba3a7a2
+++ b/bgpd/bgp_attr.c
ba3a7a2
@@ -2147,6 +2147,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ba3a7a2
   memset (seen, 0, BGP_ATTR_BITMAP_SIZE);
ba3a7a2
 
ba3a7a2
   /* End pointer of BGP attribute. */
ba3a7a2
+  assert (size <= stream_get_size (BGP_INPUT (peer)));
ba3a7a2
+  assert (size <= stream_get_endp (BGP_INPUT (peer)));
ba3a7a2
   endp = BGP_INPUT_PNT (peer) + size;
ba3a7a2
   
ba3a7a2
   /* Get attributes to the end of attribute length. */
ba3a7a2
@@ -2228,7 +2230,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ba3a7a2
           bgp_notify_send_with_data (peer,
ba3a7a2
                                      BGP_NOTIFY_UPDATE_ERR,
ba3a7a2
                                      BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
ba3a7a2
-                                     startp, attr_endp - startp);
ba3a7a2
+                                     startp, endp - startp);
ba3a7a2
 	  return BGP_ATTR_PARSE_ERROR;
ba3a7a2
 	}
ba3a7a2
 	
ba3a7a2
-- 
ba3a7a2
2.14.3
ba3a7a2