diff mbox series

[meta-networking,scarthgap,4/5] frr: fix CVE-2024-31948

Message ID 20241126081116.2535308-4-peng.zhang1.cn@windriver.com
State New
Headers show
Series [meta-networking,scarthgap,1/5] frr: fix CVE-2024-34088 | expand

Commit Message

Peng Zhang Nov. 26, 2024, 8:11 a.m. UTC
From: Zhang Peng <peng.zhang1.cn@windriver.com>

CVE-2024-31948:
In FRRouting (FRR) through 9.1, an attacker using a malformed Prefix SID attribute
in a BGP UPDATE packet can cause the bgpd daemon to crash.

Reference:
[https://nvd.nist.gov/vuln/detail/CVE-2024-31948]

Upstream patches:
[https://github.com/FRRouting/frr/commit/ba6a8f1a31e1a88df2de69ea46068e8bd9b97138]
[https://github.com/FRRouting/frr/commit/babb23b74855e23c987a63f8256d24e28c044d07]

Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
---
 .../frr/frr/CVE-2024-31948.patch              | 130 ++++++++++++++++++
 .../recipes-protocols/frr/frr_9.1.bb          |   1 +
 2 files changed, 131 insertions(+)
 create mode 100644 meta-networking/recipes-protocols/frr/frr/CVE-2024-31948.patch
diff mbox series

Patch

diff --git a/meta-networking/recipes-protocols/frr/frr/CVE-2024-31948.patch b/meta-networking/recipes-protocols/frr/frr/CVE-2024-31948.patch
new file mode 100644
index 0000000000..bc1f2edc7d
--- /dev/null
+++ b/meta-networking/recipes-protocols/frr/frr/CVE-2024-31948.patch
@@ -0,0 +1,130 @@ 
+From a11446687169c679b5e51b57f151a6f6c119656c Mon Sep 17 00:00:00 2001
+From: Donatas Abraitis <donatas@opensourcerouting.org>
+Date: Wed, 27 Mar 2024 18:42:56 +0200
+Subject: [PATCH 1/2] bgpd: Fix error handling when receiving BGP Prefix SID
+ attribute
+
+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 <iggyfran@amazon.com>
+Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
+
+CVE: CVE-2024-31948
+Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/ba6a8f1a31e1a88df2de69ea46068e8bd9b97138]
+
+Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
+---
+ 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 56e77eb3a..2639ff864 100644
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -1390,6 +1390,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
+@@ -3144,8 +3145,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);
+@@ -3195,6 +3194,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;
+ }
+ 
+-- 
+2.34.1
+
+From 70555e1c0927b84f3aae9406379b00c976b2fa0c Mon Sep 17 00:00:00 2001
+From: Donatas Abraitis <donatas@opensourcerouting.org>
+Date: Wed, 27 Mar 2024 19:08:38 +0200
+Subject: [PATCH 2/2] bgpd: Prevent from one more CVE triggering this place
+
+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 <donatas@opensourcerouting.org>
+
+CVE: CVE-2024-31948
+Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/babb23b74855e23c987a63f8256d24e28c044d07]
+
+Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
+---
+ 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 2639ff864..797f05d60 100644
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -1381,6 +1381,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
+@@ -1418,19 +1427,21 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode,
+ 					  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.
+-- 
+2.34.1
+
diff --git a/meta-networking/recipes-protocols/frr/frr_9.1.bb b/meta-networking/recipes-protocols/frr/frr_9.1.bb
index 807e4ef8ef..7043cad0f6 100644
--- a/meta-networking/recipes-protocols/frr/frr_9.1.bb
+++ b/meta-networking/recipes-protocols/frr/frr_9.1.bb
@@ -16,6 +16,7 @@  SRC_URI = "git://github.com/FRRouting/frr.git;protocol=https;branch=stable/9.1 \
            file://CVE-2024-34088.patch \
            file://CVE-2024-31950.patch \
            file://CVE-2024-31951.patch \
+           file://CVE-2024-31948.patch \
            "
 
 SRCREV = "ca2d6f0f1e000951224a18973cc1827f7f5215b5"