From 728032ef78e217142d6664aea325a399e9a7ef5a5aa0fe03a867f585d514c3f5 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Thu, 11 Apr 2024 14:28:22 +0000 Subject: [PATCH] Accepting request 1166797 from home:cfconrad:branches:network - Apply upstream fix on error handling when receiving BGP Prefix SID attribute (bsc#1222518,CVE-2024-31948,gh#FRRouting/frr#15628) OBS-URL: https://build.opensuse.org/request/show/1166797 OBS-URL: https://build.opensuse.org/package/show/network/frr?expand=0&rev=62 --- ...n-receiving-BGP-Prefix-SID-attribute.patch | 121 ++++++++++++++++++ frr.changes | 6 + frr.spec | 3 +- 3 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 0019-bgpd-fix-error-handling-when-receiving-BGP-Prefix-SID-attribute.patch diff --git a/0019-bgpd-fix-error-handling-when-receiving-BGP-Prefix-SID-attribute.patch b/0019-bgpd-fix-error-handling-when-receiving-BGP-Prefix-SID-attribute.patch new file mode 100644 index 0000000..624015b --- /dev/null +++ b/0019-bgpd-fix-error-handling-when-receiving-BGP-Prefix-SID-attribute.patch @@ -0,0 +1,121 @@ +From 51679e4504546584d98673b76ed8e12a8bc74fe0 Mon Sep 17 00:00:00 2001 +From: Donatas Abraitis +Date: Wed, 27 Mar 2024 18:42:56 +0200 +Subject: [PATCH 1/2] bgpd: Fix error handling when receiving BGP Prefix SID + attribute +References: bsc#1222518 CVE-2024-31948 gh#FRRouting/frr#15628 + + +Without this patch, we always set the BGP Prefix SID attribute flag without +checking if it's malformed or not. RFC8669 says that this attribute MUST be discarded. + +Also, this fixes the bgpd crash when a malformed Prefix SID attribute is received, +with malformed transitive flags and/or TLVs. + +Reported-by: Iggy Frankovic +Signed-off-by: Donatas Abraitis +(cherry picked from commit ba6a8f1a31e1a88df2de69ea46068e8bd9b97138) +--- + bgpd/bgp_attr.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c +index 7144c4bfa73d..2e2845b8fa7e 100644 +--- a/bgpd/bgp_attr.c ++++ b/bgpd/bgp_attr.c +@@ -1400,6 +1400,7 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, + case BGP_ATTR_AS4_AGGREGATOR: + case BGP_ATTR_AGGREGATOR: + case BGP_ATTR_ATOMIC_AGGREGATE: ++ case BGP_ATTR_PREFIX_SID: + return BGP_ATTR_PARSE_PROCEED; + + /* Core attributes, particularly ones which may influence route +@@ -3146,8 +3147,6 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args) + struct attr *const attr = args->attr; + enum bgp_attr_parse_ret ret; + +- attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID); +- + uint8_t type; + uint16_t length; + size_t headersz = sizeof(type) + sizeof(length); +@@ -3197,6 +3196,8 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args) + } + } + ++ SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID)); ++ + return BGP_ATTR_PARSE_PROCEED; + } + + +From 9240abccb564043c85180916b77cad5b194a49c9 Mon Sep 17 00:00:00 2001 +From: Donatas Abraitis +Date: Wed, 27 Mar 2024 19:08:38 +0200 +Subject: [PATCH 2/2] bgpd: Prevent from one more CVE triggering this place +References: bsc#1222518 CVE-2024-31948 gh#FRRouting/frr#15628 +Upstream: submitted + +If we receive an attribute that is handled by bgp_attr_malformed(), use +treat-as-withdraw behavior for unknown (or missing to add - if new) attributes. + +Signed-off-by: Donatas Abraitis +(cherry picked from commit babb23b74855e23c987a63f8256d24e28c044d07) +--- + bgpd/bgp_attr.c | 33 ++++++++++++++++++++++----------- + 1 file changed, 22 insertions(+), 11 deletions(-) + +diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c +index 2e2845b8fa7e..7570598a3d7f 100644 +--- a/bgpd/bgp_attr.c ++++ b/bgpd/bgp_attr.c +@@ -1391,6 +1391,15 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, + (args->startp - STREAM_DATA(BGP_INPUT(peer))) + + args->total); + ++ /* Partial optional attributes that are malformed should not cause ++ * the whole session to be reset. Instead treat it as a withdrawal ++ * of the routes, if possible. ++ */ ++ if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS) && ++ CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL) && ++ CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL)) ++ return BGP_ATTR_PARSE_WITHDRAW; ++ + switch (args->type) { + /* where an attribute is relatively inconsequential, e.g. it does not + * affect route selection, and can be safely ignored, then any such +@@ -1425,19 +1434,21 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, + bgp_notify_send_with_data(peer, BGP_NOTIFY_UPDATE_ERR, subcode, + notify_datap, length); + return BGP_ATTR_PARSE_ERROR; ++ default: ++ /* Unknown attributes, that are handled by this function ++ * should be treated as withdraw, to prevent one more CVE ++ * from being introduced. ++ * RFC 7606 says: ++ * The "treat-as-withdraw" approach is generally preferred ++ * and the "session reset" approach is discouraged. ++ */ ++ flog_err(EC_BGP_ATTR_FLAG, ++ "%s(%u) attribute received, while it is not known how to handle it, treating as withdraw", ++ lookup_msg(attr_str, args->type, NULL), args->type); ++ break; + } + +- /* Partial optional attributes that are malformed should not cause +- * the whole session to be reset. Instead treat it as a withdrawal +- * of the routes, if possible. +- */ +- if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS) +- && CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL) +- && CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL)) +- return BGP_ATTR_PARSE_WITHDRAW; +- +- /* default to reset */ +- return BGP_ATTR_PARSE_ERROR_NOTIFYPLS; ++ return BGP_ATTR_PARSE_WITHDRAW; + } + + /* Find out what is wrong with the path attribute flag bits and log the error. diff --git a/frr.changes b/frr.changes index 8fbb87d..1394fcf 100644 --- a/frr.changes +++ b/frr.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Apr 10 18:59:00 UTC 2024 - Clemens Famulla-Conrad + +- Apply upstream fix on error handling when receiving BGP Prefix + SID attribute (bsc#1222518,CVE-2024-31948,gh#FRRouting/frr#15628) + ------------------------------------------------------------------- Thu Feb 8 06:55:28 UTC 2024 - Dominique Leuenberger diff --git a/frr.spec b/frr.spec index 3ecbe6d..e88f4c4 100644 --- a/frr.spec +++ b/frr.spec @@ -57,6 +57,7 @@ Patch15: 0015-bgpd-Treat-EOR-as-withdrawn-to-avoid-unwanted-handli.patch Patch16: 0016-bgpd-Ignore-handling-NLRIs-if-we-received-MP_UNREACH.patch Patch17: 0017-bgpd-Fix-use-beyond-end-of-stream-of-labeled-unicast.patch Patch18: 0018-bgpd-Flowspec-overflow-issue.patch +Patch19: 0019-bgpd-fix-error-handling-when-receiving-BGP-Prefix-SID-attribute.patch BuildRequires: autoconf BuildRequires: automake BuildRequires: bison >= 2.7 @@ -92,7 +93,7 @@ BuildRequires: pkgconfig(sqlite3) Requires(post): %{install_info_prereq} Requires(pre): %{install_info_prereq} Requires(pre): shadow -Requires(preun): %{install_info_prereq} +Requires(preun):%{install_info_prereq} Recommends: logrotate Conflicts: quagga Provides: zebra = %{version}