|
|
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 |
|