157a198d30
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>
113 lines
3.4 KiB
Diff
113 lines
3.4 KiB
Diff
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
|
|
|