New version 1.2.4

This commit is contained in:
Michal Ruprich 2018-05-04 17:46:57 +02:00
parent ba3a7a25ca
commit 7ba3ea9c42
7 changed files with 9 additions and 343 deletions

2
.gitignore vendored
View File

@ -15,3 +15,5 @@ quagga-0.99.17.tar.gz
/quagga-1.2.1.tar.gz
/quagga-1.2.2.tar.gz
/quagga-1.2.2.tar.gz.asc
/quagga-1.2.4.tar.gz
/quagga-1.2.4.tar.gz.asc

View File

@ -1,110 +0,0 @@
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.
---
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.14.3

View File

@ -1,112 +0,0 @@
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.
---
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.14.3

View File

@ -1,41 +0,0 @@
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.
---
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.14.3

View File

@ -1,67 +0,0 @@
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.
---
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.14.3

View File

@ -6,8 +6,8 @@
%global _hardened_build 1
Name: quagga
Version: 1.2.2
Release: 4%{?dist}
Version: 1.2.4
Release: 1%{?dist}
Summary: Routing daemon
License: GPLv2+
Group: System Environment/Daemons
@ -29,15 +29,6 @@ Requires(postun): systemd
Provides: routingdaemon = %{version}-%{release}
Obsoletes: quagga-sysvinit
# Upstream patch:
Patch0: 0001-bgpd-security-Fix-double-free-of-unknown-attribute.patch
# Upstream patch:
Patch1: 0001-bgpd-security-debug-print-of-received-NOTIFY-data-ca.patch
# Upstream patch:
Patch2: 0001-bgpd-security-fix-infinite-loop-on-certain-invalid-O.patch
# Upstream patch:
Patch3: 0001-bgpd-security-invalid-attr-length-sends-NOTIFY-with-.patch
%define __perl_requires %{SOURCE1}
%description
@ -234,6 +225,9 @@ fi
%{_includedir}/quagga/ospfd/*.h
%changelog
* Fri May 04 2018 Michal Ruprich <mruprich@redhat.com> - 1.2.4-1
- New version 1.2.4
* Thu Feb 22 2018 Ondřej Lysoněk <olysonek@redhat.com> - 1.2.2-4
- Fixed CVE-2018-5379 - Double free vulnerability in bgpd when processing
certain forms of UPDATE message allowing to crash or potentially execute

View File

@ -1,2 +1,2 @@
SHA512 (quagga-1.2.2.tar.gz) = 861f6524bcdc01d1a895762bf1904744c12ae4dfc7c3583ecb7e55b3978c98187bde76df0ff85093c744139be9d5cf324fec75b5ba86cf1fdbce70d923710d14
SHA512 (quagga-1.2.2.tar.gz.asc) = bb88e1a598f585255700bd7362ffed8ce3a0697c7df22747da27ba28ed43b400ee8ce5920cc90229359cc217cb6bac41bf546c259b1cfbab2943680cb177e52d
SHA512 (quagga-1.2.4.tar.gz) = 3e72440bcccfd3c1a449a62b7ff8623441256399a2bee0a39fa0a19694a5a78ac909c5c2128a24735bc034ea8b0811827293b480a2584a3a4c8ae36be9cf1fcd
SHA512 (quagga-1.2.4.tar.gz.asc) = 054f6159bf3e2ea396e696d6297b026d1322b17eba31826cf3ac42b5a43e924caef1d87bba481cc3c272b56aa5c64b3d5537a67693f99cafb560d216870fede3