From 839ea37939ad1c6cbb4ac543962bbb66be4a2a3f Mon Sep 17 00:00:00 2001 From: Josef Schlehofer Date: Sun, 26 Jan 2020 23:13:23 +0100 Subject: [PATCH] quagga: update to version 1.1.1 (#541) Makefile changes: - Use HTTPS everywhere - Reorder some things to have it in sync with other packages - Fixed SPDX License Identifier - Added PKG_LICENSE_FILES - For checksum use SHA256 Refreshed patches Fixes CVEs: - CVE-2017-5495 - CVE-2017-16227 Fixes security issues: - Quagga-2018-0543: attr_endp used for NOTIFY data https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-0543.txt - Quagga-2018-1114: bgpd double free https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1114.txt - Quagga-2018-1550: debug overrun in notify lookup tables https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1550.txt - Quagga-2018-1975: BGP capability inf. loop https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1975.txt Signed-off-by: Josef Schlehofer --- quagga/Makefile | 22 ++-- ...PATH-size-calculation-for-long-paths.patch | 28 +++++ quagga/patches/002-Quagga-2018-0543.patch | 58 ++++++++++ quagga/patches/003-Quagga-2018-1114.patch | 99 +++++++++++++++++ quagga/patches/004-Quagga-2018-1550.patch | 104 ++++++++++++++++++ quagga/patches/005-Quagga-2018-1975.patch | 32 ++++++ quagga/patches/150-no-cross-fs-link.patch | 4 +- 7 files changed, 336 insertions(+), 11 deletions(-) create mode 100644 quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch create mode 100644 quagga/patches/002-Quagga-2018-0543.patch create mode 100644 quagga/patches/003-Quagga-2018-1114.patch create mode 100644 quagga/patches/004-Quagga-2018-1550.patch create mode 100644 quagga/patches/005-Quagga-2018-1975.patch diff --git a/quagga/Makefile b/quagga/Makefile index e2751f4..91fee84 100644 --- a/quagga/Makefile +++ b/quagga/Makefile @@ -8,12 +8,21 @@ include $(TOPDIR)/rules.mk PKG_NAME:=quagga -PKG_VERSION:=1.1.0 +PKG_VERSION:=1.1.1 PKG_RELEASE:=1 -PKG_HASH:=f7a43a9c59bfd3722002210530b2553c8d5cc05bfea5acd56d4f102b9f55dc63 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz PKG_SOURCE_URL:=@SAVANNAH/quagga/ +PKG_HASH:=b5a94e5bdad3062e04595a5692b8cc435f0a85102f75dfdca0a06d093b4ef63f + +PKG_MAINTAINER:=Vasilis Tsiligiannis +PKG_LICENSE:=GPL-2.0-or-later +PKG_LICENSE_FILES:=COPYING + +PKG_BUILD_PARALLEL:=1 +PKG_FIXUP:=autoreconf +PKG_INSTALL:=1 + PKG_CONFIG_DEPENDS:= \ CONFIG_IPV6 \ CONFIG_PACKAGE_quagga-watchquagga \ @@ -26,10 +35,6 @@ PKG_CONFIG_DEPENDS:= \ CONFIG_PACKAGE_quagga-ripd \ CONFIG_PACKAGE_quagga-ripngd \ CONFIG_PACKAGE_quagga-vtysh -PKG_BUILD_PARALLEL:=1 -PKG_FIXUP:=autoreconf -PKG_INSTALL:=1 -PKG_LICENSE:=GPL-2.0 include $(INCLUDE_DIR)/package.mk @@ -37,10 +42,9 @@ define Package/quagga/Default SECTION:=net CATEGORY:=Network SUBMENU:=Routing and Redirection - DEPENDS:=quagga TITLE:=The Quagga Software Routing Suite - URL:=http://www.quagga.net - MAINTAINER:=Vasilis Tsiligiannis + URL:=https://www.quagga.net + DEPENDS:=quagga endef define Package/quagga diff --git a/quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch b/quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch new file mode 100644 index 0000000..3ecbc0e --- /dev/null +++ b/quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch @@ -0,0 +1,28 @@ +From: Andreas Jaggi +Date: Mon, 2 Oct 2017 19:38:43 +0530 +Subject: bgpd: Fix AS_PATH size calculation for long paths +Origin: http://git.savannah.gnu.org/cgit/quagga.git/commit?id=7a42b78be9a4108d98833069a88e6fddb9285008 +Bug-Debian: https://bugs.debian.org/879474 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-16227 + +If you have an AS_PATH with more entries than +what can be written into a single AS_SEGMENT_MAX +it needs to be broken up. The code that noticed +that the AS_PATH needs to be broken up was not +correctly calculating the size of the resulting +message. This patch addresses this issue. +--- + bgpd/bgp_aspath.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/bgpd/bgp_aspath.c ++++ b/bgpd/bgp_aspath.c +@@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct asp + assegment_header_put (s, seg->type, AS_SEGMENT_MAX); + assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit); + written += AS_SEGMENT_MAX; +- bytes += ASSEGMENT_SIZE (written, use32bit); ++ bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit); + } + + /* write the final segment, probably is also the first */ diff --git a/quagga/patches/002-Quagga-2018-0543.patch b/quagga/patches/002-Quagga-2018-0543.patch new file mode 100644 index 0000000..1f77143 --- /dev/null +++ b/quagga/patches/002-Quagga-2018-0543.patch @@ -0,0 +1,58 @@ +From cc2e6770697e343f4af534114ab7e633d5beabec Mon Sep 17 00:00:00 2001 +From: Paul Jakma +Date: Wed, 3 Jan 2018 23:57:33 +0000 +Subject: 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. + +--- a/bgpd/bgp_attr.c ++++ b/bgpd/bgp_attr.c +@@ -2079,6 +2079,8 @@ bgp_attr_parse (struct peer *peer, struc + 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. */ +@@ -2160,7 +2162,7 @@ bgp_attr_parse (struct peer *peer, struc + 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; + } + diff --git a/quagga/patches/003-Quagga-2018-1114.patch b/quagga/patches/003-Quagga-2018-1114.patch new file mode 100644 index 0000000..a4b91c5 --- /dev/null +++ b/quagga/patches/003-Quagga-2018-1114.patch @@ -0,0 +1,99 @@ +From e69b535f92eafb599329bf725d9b4c6fd5d7fded Mon Sep 17 00:00:00 2001 +From: Paul Jakma +Date: Sat, 6 Jan 2018 19:52:10 +0000 +Subject: 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. + +--- a/bgpd/bgp_attr.c ++++ b/bgpd/bgp_attr.c +@@ -186,15 +186,17 @@ cluster_intern (struct cluster_list *clu + } + + 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; + } + } + +@@ -344,15 +346,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; + } + } + +@@ -793,11 +798,11 @@ bgp_attr_unintern_sub (struct attr *attr + UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_EXT_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); + } + } + +--- a/bgpd/bgp_attr.h ++++ b/bgpd/bgp_attr.h +@@ -184,10 +184,10 @@ extern unsigned long int attr_unknown_co + + /* 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 { diff --git a/quagga/patches/004-Quagga-2018-1550.patch b/quagga/patches/004-Quagga-2018-1550.patch new file mode 100644 index 0000000..4bd3da8 --- /dev/null +++ b/quagga/patches/004-Quagga-2018-1550.patch @@ -0,0 +1,104 @@ +From 9e5251151894aefdf8e9392a2371615222119ad8 Mon Sep 17 00:00:00 2001 +From: Paul Jakma +Date: Sat, 6 Jan 2018 22:31:52 +0000 +Subject: 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. + +--- a/bgpd/bgp_debug.c ++++ b/bgpd/bgp_debug.c +@@ -29,6 +29,7 @@ Software Foundation, Inc., 59 Temple Pla + #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_m + { 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_o + { 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_u + { 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_c + { 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_c + { 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","?"}; diff --git a/quagga/patches/005-Quagga-2018-1975.patch b/quagga/patches/005-Quagga-2018-1975.patch new file mode 100644 index 0000000..5e0c6ee --- /dev/null +++ b/quagga/patches/005-Quagga-2018-1975.patch @@ -0,0 +1,32 @@ +From ce07207c50a3d1f05d6dd49b5294282e59749787 Mon Sep 17 00:00:00 2001 +From: Paul Jakma +Date: Sat, 6 Jan 2018 21:20:51 +0000 +Subject: 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. + +--- a/bgpd/bgp_packet.c ++++ b/bgpd/bgp_packet.c +@@ -2251,7 +2251,8 @@ bgp_capability_msg_parse (struct peer *p + + 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) +@@ -2339,7 +2340,6 @@ bgp_capability_msg_parse (struct peer *p + zlog_warn ("%s unrecognized capability code: %d - ignored", + peer->host, hdr->code); + } +- pnt += hdr->length + 3; + } + return 0; + } diff --git a/quagga/patches/150-no-cross-fs-link.patch b/quagga/patches/150-no-cross-fs-link.patch index 8445103..18078bc 100644 --- a/quagga/patches/150-no-cross-fs-link.patch +++ b/quagga/patches/150-no-cross-fs-link.patch @@ -1,6 +1,6 @@ --- a/lib/command.c +++ b/lib/command.c -@@ -3198,6 +3198,13 @@ DEFUN (config_write_file, +@@ -3200,6 +3200,13 @@ DEFUN (config_write_file, VTY_NEWLINE); goto finished; } @@ -14,7 +14,7 @@ if (link (config_file, config_file_sav) != 0) { vty_out (vty, "Can't backup old configuration file %s.%s", config_file_sav, -@@ -3211,7 +3218,23 @@ DEFUN (config_write_file, +@@ -3213,7 +3220,23 @@ DEFUN (config_write_file, VTY_NEWLINE); goto finished; }