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