frr: fix CVE-2024-31948

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>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
This commit is contained in:
Zhang Peng
2024-11-26 16:11:15 +08:00
committed by Armin Kuster
parent 483946a97b
commit 2d7769f90b
2 changed files with 131 additions and 0 deletions
@@ -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
@@ -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"