* [Buildroot] [git commit] quagga: add upstream security fixes
@ 2018-02-19 22:48 Peter Korsgaard
0 siblings, 0 replies; only message in thread
From: Peter Korsgaard @ 2018-02-19 22:48 UTC (permalink / raw)
To: buildroot
commit: https://git.buildroot.net/buildroot/commit/?id=157a198d304224c12fa0d91d977a6619d021f5c6
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
Fixes the following security issues:
CVE-2018-5378
It was discovered that the Quagga BGP daemon, bgpd, does not
properly bounds check data sent with a NOTIFY to a peer, if an
attribute length is invalid. A configured BGP peer can take
advantage of this bug to read memory from the bgpd process or cause
a denial of service (daemon crash).
https://www.quagga.net/security/Quagga-2018-0543.txt
CVE-2018-5379
It was discovered that the Quagga BGP daemon, bgpd, can double-free
memory when processing certain forms of UPDATE message, containing
cluster-list and/or unknown attributes, resulting in a denial of
service (bgpd daemon crash).
https://www.quagga.net/security/Quagga-2018-1114.txt
CVE-2018-5380
It was discovered that the Quagga BGP daemon, bgpd, does not
properly handle internal BGP code-to-string conversion tables.
https://www.quagga.net/security/Quagga-2018-1550.txt
CVE-2018-5381
It was discovered that the Quagga BGP daemon, bgpd, can enter an
infinite loop if sent an invalid OPEN message by a configured peer.
A configured peer can take advantage of this flaw to cause a denial
of service (bgpd daemon not responding to any other events; BGP
sessions will drop and not be reestablished; unresponsive CLI
interface).
https://www.quagga.net/security/Quagga-2018-1975.txt
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
...ty-invalid-attr-length-sends-NOTIFY-with-.patch | 69 +++++++++++++
...rity-Fix-double-free-of-unknown-attribute.patch | 112 ++++++++++++++++++++
...ty-debug-print-of-received-NOTIFY-data-ca.patch | 114 +++++++++++++++++++++
...ty-fix-infinite-loop-on-certain-invalid-O.patch | 43 ++++++++
4 files changed, 338 insertions(+)
diff --git a/package/quagga/0005-bgpd-security-invalid-attr-length-sends-NOTIFY-with-.patch b/package/quagga/0005-bgpd-security-invalid-attr-length-sends-NOTIFY-with-.patch
new file mode 100644
index 0000000000..b64109d0f7
--- /dev/null
+++ b/package/quagga/0005-bgpd-security-invalid-attr-length-sends-NOTIFY-with-.patch
@@ -0,0 +1,69 @@
+From cc2e6770697e343f4af534114ab7e633d5beabec Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Wed, 3 Jan 2018 23:57:33 +0000
+Subject: [PATCH] bgpd/security: invalid attr length sends NOTIFY with data
+ overrun
+
+Security issue: Quagga-2018-0543
+
+See: https://www.quagga.net/security/Quagga-2018-0543.txt
+
+* bgpd/bgp_attr.c: (bgp_attr_parse) An invalid attribute length is correctly
+ checked, and a NOTIFY prepared. The NOTIFY can include the incorrect
+ received data with the NOTIFY, for debug purposes. Commit
+ c69698704806a9ac5 modified the code to do that just, and also send the
+ malformed attr with the NOTIFY. However, the invalid attribute length was
+ used as the length of the data to send back.
+
+ The result is a read past the end of data, which is then written to the
+ NOTIFY message and sent to the peer.
+
+ A configured BGP peer can use this bug to read up to 64 KiB of memory from
+ the bgpd process, or crash the process if the invalid read is caught by
+ some means (unmapped page and SEGV, or other mechanism) resulting in a DoS.
+
+ This bug _ought_ /not/ be exploitable by anything other than the connected
+ BGP peer, assuming the underlying TCP transport is secure. For no BGP
+ peer should send on an UPDATE with this attribute. Quagga will not, as
+ Quagga always validates the attr header length, regardless of type.
+
+ However, it is possible that there are BGP implementations that do not
+ check lengths on some attributes (e.g. optional/transitive ones of a type
+ they do not recognise), and might pass such malformed attrs on. If such
+ implementations exists and are common, then this bug might be triggerable
+ by BGP speakers further hops away. Those peers will not receive the
+ NOTIFY (unless they sit on a shared medium), however they might then be
+ able to trigger a DoS.
+
+ Fix: use the valid bound to calculate the length.
+
+Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
+---
+ bgpd/bgp_attr.c | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
+index ef58beb1..9564637e 100644
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -2147,6 +2147,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
+ memset (seen, 0, BGP_ATTR_BITMAP_SIZE);
+
+ /* End pointer of BGP attribute. */
++ assert (size <= stream_get_size (BGP_INPUT (peer)));
++ assert (size <= stream_get_endp (BGP_INPUT (peer)));
+ endp = BGP_INPUT_PNT (peer) + size;
+
+ /* Get attributes to the end of attribute length. */
+@@ -2228,7 +2230,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
+ bgp_notify_send_with_data (peer,
+ BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+- startp, attr_endp - startp);
++ startp, endp - startp);
+ return BGP_ATTR_PARSE_ERROR;
+ }
+
+--
+2.11.0
+
diff --git a/package/quagga/0006-bgpd-security-Fix-double-free-of-unknown-attribute.patch b/package/quagga/0006-bgpd-security-Fix-double-free-of-unknown-attribute.patch
new file mode 100644
index 0000000000..0e32817f06
--- /dev/null
+++ b/package/quagga/0006-bgpd-security-Fix-double-free-of-unknown-attribute.patch
@@ -0,0 +1,112 @@
+From e69b535f92eafb599329bf725d9b4c6fd5d7fded Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 19:52:10 +0000
+Subject: [PATCH] bgpd/security: Fix double free of unknown attribute
+
+Security issue: Quagga-2018-1114
+See: https://www.quagga.net/security/Quagga-2018-1114.txt
+
+It is possible for bgpd to double-free an unknown attribute. This can happen
+via bgp_update_receive receiving an UPDATE with an invalid unknown attribute.
+bgp_update_receive then will call bgp_attr_unintern_sub and bgp_attr_flush,
+and the latter may try free an already freed unknown attr.
+
+* bgpd/bgp_attr.c: (transit_unintern) Take a pointer to the caller's storage
+ for the (struct transit *), so that transit_unintern can NULL out the
+ caller's reference if the (struct transit) is freed.
+ (cluster_unintern) By inspection, appears to have a similar issue.
+ (bgp_attr_unintern_sub) adjust for above.
+
+Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
+---
+ bgpd/bgp_attr.c | 33 +++++++++++++++++++--------------
+ bgpd/bgp_attr.h | 4 ++--
+ 2 files changed, 21 insertions(+), 16 deletions(-)
+
+diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
+index 9564637e..0c2806b5 100644
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -199,15 +199,17 @@ cluster_intern (struct cluster_list *cluster)
+ }
+
+ void
+-cluster_unintern (struct cluster_list *cluster)
++cluster_unintern (struct cluster_list **cluster)
+ {
+- if (cluster->refcnt)
+- cluster->refcnt--;
++ struct cluster_list *c = *cluster;
++ if (c->refcnt)
++ c->refcnt--;
+
+- if (cluster->refcnt == 0)
++ if (c->refcnt == 0)
+ {
+- hash_release (cluster_hash, cluster);
+- cluster_free (cluster);
++ hash_release (cluster_hash, c);
++ cluster_free (c);
++ *cluster = NULL;
+ }
+ }
+
+@@ -357,15 +359,18 @@ transit_intern (struct transit *transit)
+ }
+
+ void
+-transit_unintern (struct transit *transit)
++transit_unintern (struct transit **transit)
+ {
+- if (transit->refcnt)
+- transit->refcnt--;
++ struct transit *t = *transit;
++
++ if (t->refcnt)
++ t->refcnt--;
+
+- if (transit->refcnt == 0)
++ if (t->refcnt == 0)
+ {
+- hash_release (transit_hash, transit);
+- transit_free (transit);
++ hash_release (transit_hash, t);
++ transit_free (t);
++ *transit = NULL;
+ }
+ }
+
+@@ -820,11 +825,11 @@ bgp_attr_unintern_sub (struct attr *attr)
+ UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LARGE_COMMUNITIES));
+
+ if (attr->extra->cluster)
+- cluster_unintern (attr->extra->cluster);
++ cluster_unintern (&attr->extra->cluster);
+ UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST));
+
+ if (attr->extra->transit)
+- transit_unintern (attr->extra->transit);
++ transit_unintern (&attr->extra->transit);
+ }
+ }
+
+diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
+index 9ff074b2..052acc7d 100644
+--- a/bgpd/bgp_attr.h
++++ b/bgpd/bgp_attr.h
+@@ -187,10 +187,10 @@ extern unsigned long int attr_unknown_count (void);
+
+ /* Cluster list prototypes. */
+ extern int cluster_loop_check (struct cluster_list *, struct in_addr);
+-extern void cluster_unintern (struct cluster_list *);
++extern void cluster_unintern (struct cluster_list **);
+
+ /* Transit attribute prototypes. */
+-void transit_unintern (struct transit *);
++void transit_unintern (struct transit **);
+
+ /* Below exported for unit-test purposes only */
+ struct bgp_attr_parser_args {
+--
+2.11.0
+
diff --git a/package/quagga/0007-bgpd-security-debug-print-of-received-NOTIFY-data-ca.patch b/package/quagga/0007-bgpd-security-debug-print-of-received-NOTIFY-data-ca.patch
new file mode 100644
index 0000000000..aeb50ae559
--- /dev/null
+++ b/package/quagga/0007-bgpd-security-debug-print-of-received-NOTIFY-data-ca.patch
@@ -0,0 +1,114 @@
+From 9e5251151894aefdf8e9392a2371615222119ad8 Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 22:31:52 +0000
+Subject: [PATCH] bgpd/security: debug print of received NOTIFY data can
+ over-read msg array
+
+Security issue: Quagga-2018-1550
+See: https://www.quagga.net/security/Quagga-2018-1550.txt
+
+* bgpd/bgp_debug.c: (struct message) Nearly every one of the NOTIFY
+ code/subcode message arrays has their corresponding size variables off
+ by one, as most have 1 as first index.
+
+ This means (bgp_notify_print) can cause mes_lookup to overread the (struct
+ message) by 1 pointer value if given an unknown index.
+
+ Fix the bgp_notify_..._msg_max variables to use the compiler to calculate
+ the correct sizes.
+
+Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
+---
+ bgpd/bgp_debug.c | 21 ++++++++++++---------
+ 1 file changed, 12 insertions(+), 9 deletions(-)
+
+diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c
+index ba797228..43faee7c 100644
+--- a/bgpd/bgp_debug.c
++++ b/bgpd/bgp_debug.c
+@@ -29,6 +29,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ #include "log.h"
+ #include "sockunion.h"
+ #include "filter.h"
++#include "memory.h"
+
+ #include "bgpd/bgpd.h"
+ #include "bgpd/bgp_aspath.h"
+@@ -73,7 +74,8 @@ const struct message bgp_status_msg[] =
+ { Clearing, "Clearing" },
+ { Deleted, "Deleted" },
+ };
+-const int bgp_status_msg_max = BGP_STATUS_MAX;
++#define BGP_DEBUG_MSG_MAX(msg) const int msg ## _max = array_size (msg)
++BGP_DEBUG_MSG_MAX (bgp_status_msg);
+
+ /* BGP message type string. */
+ const char *bgp_type_str[] =
+@@ -84,7 +86,8 @@ const char *bgp_type_str[] =
+ "NOTIFICATION",
+ "KEEPALIVE",
+ "ROUTE-REFRESH",
+- "CAPABILITY"
++ "CAPABILITY",
++ NULL,
+ };
+
+ /* message for BGP-4 Notify */
+@@ -98,15 +101,15 @@ static const struct message bgp_notify_msg[] =
+ { BGP_NOTIFY_CEASE, "Cease"},
+ { BGP_NOTIFY_CAPABILITY_ERR, "CAPABILITY Message Error"},
+ };
+-static const int bgp_notify_msg_max = BGP_NOTIFY_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_msg);
+
+ static const struct message bgp_notify_head_msg[] =
+ {
+ { BGP_NOTIFY_HEADER_NOT_SYNC, "/Connection Not Synchronized"},
+ { BGP_NOTIFY_HEADER_BAD_MESLEN, "/Bad Message Length"},
+- { BGP_NOTIFY_HEADER_BAD_MESTYPE, "/Bad Message Type"}
++ { BGP_NOTIFY_HEADER_BAD_MESTYPE, "/Bad Message Type"},
+ };
+-static const int bgp_notify_head_msg_max = BGP_NOTIFY_HEADER_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_head_msg);
+
+ static const struct message bgp_notify_open_msg[] =
+ {
+@@ -119,7 +122,7 @@ static const struct message bgp_notify_open_msg[] =
+ { BGP_NOTIFY_OPEN_UNACEP_HOLDTIME, "/Unacceptable Hold Time"},
+ { BGP_NOTIFY_OPEN_UNSUP_CAPBL, "/Unsupported Capability"},
+ };
+-static const int bgp_notify_open_msg_max = BGP_NOTIFY_OPEN_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_open_msg);
+
+ static const struct message bgp_notify_update_msg[] =
+ {
+@@ -136,7 +139,7 @@ static const struct message bgp_notify_update_msg[] =
+ { BGP_NOTIFY_UPDATE_INVAL_NETWORK, "/Invalid Network Field"},
+ { BGP_NOTIFY_UPDATE_MAL_AS_PATH, "/Malformed AS_PATH"},
+ };
+-static const int bgp_notify_update_msg_max = BGP_NOTIFY_UPDATE_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_update_msg);
+
+ static const struct message bgp_notify_cease_msg[] =
+ {
+@@ -150,7 +153,7 @@ static const struct message bgp_notify_cease_msg[] =
+ { BGP_NOTIFY_CEASE_COLLISION_RESOLUTION, "/Connection collision resolution"},
+ { BGP_NOTIFY_CEASE_OUT_OF_RESOURCE, "/Out of Resource"},
+ };
+-static const int bgp_notify_cease_msg_max = BGP_NOTIFY_CEASE_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_cease_msg);
+
+ static const struct message bgp_notify_capability_msg[] =
+ {
+@@ -159,7 +162,7 @@ static const struct message bgp_notify_capability_msg[] =
+ { BGP_NOTIFY_CAPABILITY_INVALID_LENGTH, "/Invalid Capability Length"},
+ { BGP_NOTIFY_CAPABILITY_MALFORMED_CODE, "/Malformed Capability Value"},
+ };
+-static const int bgp_notify_capability_msg_max = BGP_NOTIFY_CAPABILITY_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_capability_msg);
+
+ /* Origin strings. */
+ const char *bgp_origin_str[] = {"i","e","?"};
+--
+2.11.0
+
diff --git a/package/quagga/0008-bgpd-security-fix-infinite-loop-on-certain-invalid-O.patch b/package/quagga/0008-bgpd-security-fix-infinite-loop-on-certain-invalid-O.patch
new file mode 100644
index 0000000000..0a06da9330
--- /dev/null
+++ b/package/quagga/0008-bgpd-security-fix-infinite-loop-on-certain-invalid-O.patch
@@ -0,0 +1,43 @@
+From ce07207c50a3d1f05d6dd49b5294282e59749787 Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 21:20:51 +0000
+Subject: [PATCH] bgpd/security: fix infinite loop on certain invalid OPEN
+ messages
+
+Security issue: Quagga-2018-1975
+See: https://www.quagga.net/security/Quagga-2018-1975.txt
+
+* bgpd/bgp_packet.c: (bgp_capability_msg_parse) capability parser can infinite
+ loop due to checks that issue 'continue' without bumping the input
+ pointer.
+
+Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
+---
+ bgpd/bgp_packet.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
+index b3d601fc..f9338d8d 100644
+--- a/bgpd/bgp_packet.c
++++ b/bgpd/bgp_packet.c
+@@ -2328,7 +2328,8 @@ bgp_capability_msg_parse (struct peer *peer, u_char *pnt, bgp_size_t length)
+
+ end = pnt + length;
+
+- while (pnt < end)
++ /* XXX: Streamify this */
++ for (; pnt < end; pnt += hdr->length + 3)
+ {
+ /* We need at least action, capability code and capability length. */
+ if (pnt + 3 > end)
+@@ -2416,7 +2417,6 @@ bgp_capability_msg_parse (struct peer *peer, u_char *pnt, bgp_size_t length)
+ zlog_warn ("%s unrecognized capability code: %d - ignored",
+ peer->host, hdr->code);
+ }
+- pnt += hdr->length + 3;
+ }
+ return 0;
+ }
+--
+2.11.0
+
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2018-02-19 22:48 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-19 22:48 [Buildroot] [git commit] quagga: add upstream security fixes Peter Korsgaard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.