frr: fix multiple CVEs

CVE-2024-27913:
ospf_te_parse_te in ospfd/ospf_te.c in FRRouting (FRR) through 9.1
allows remote attackers to cause a denial of service (ospfd daemon
 crash) via a malformed OSPF LSA packet, because of an attempted
access to a missing attribute field.

CVE-2024-34088:
In FRRouting (FRR) through 9.1, it is possible for the get_edge()
function in ospf_te.c in the OSPF daemon to return a NULL pointer.
In cases where calling functions do not handle the returned NULL
value, the OSPF daemon crashes, leading to denial of service.

CVE-2024-31950:
In FRRouting (FRR) through 9.1, there can be a buffer overflow and
daemon crash in ospf_te_parse_ri for OSPF LSA packets during an attempt
to read Segment Routing subTLVs (their size is not validated).

CVE-2024-31951:
In the Opaque LSA Extended Link parser in FRRouting (FRR) through 9.1,
there can be a buffer overflow and daemon crash in
ospf_te_parse_ext_link for OSPF LSA packets during an attempt to read
Segment Routing Adjacency SID subTLVs (lengths are not validated).

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-27913]
[https://nvd.nist.gov/vuln/detail/CVE-2024-34088]
[https://nvd.nist.gov/vuln/detail/CVE-2024-31951]
[https://nvd.nist.gov/vuln/detail/CVE-2024-31950]
[https://nvd.nist.gov/vuln/detail/CVE-2024-31948]

Upstream patches:
[a73e66d073]
[8c177d69e3]
[5557a289ac]
[f69d1313b1]
[babb23b748]
[ba6a8f1a31]

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-18 18:03:14 +08:00 committed by Armin Kuster
parent feb3793070
commit 84ebedfcf4
6 changed files with 443 additions and 1 deletions

View File

@ -0,0 +1,43 @@
From d2dda70be42402e0d456e1ead4035e196253f77f Mon Sep 17 00:00:00 2001
From: Olivier Dugeon <olivier.dugeon@orange.com>
Date: Mon, 26 Feb 2024 10:40:34 +0100
Subject: [PATCH] ospfd: Solved crash in OSPF TE parsing
Iggy Frankovic discovered an ospfd crash when perfomring fuzzing of OSPF LSA
packets. The crash occurs in ospf_te_parse_te() function when attemping to
create corresponding egde from TE Link parameters. If there is no local
address, an edge is created but without any attributes. During parsing, the
function try to access to this attribute fields which has not been created
causing an ospfd crash.
The patch simply check if the te parser has found a valid local address. If not
found, we stop the parser which avoid the crash.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
CVE: CVE-2024-27913
Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/a73e66d07329d721f26f3f336f7735de420b0183]
Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
---
ospfd/ospf_te.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 999bc49d9..5af006e54 100644
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -2276,6 +2276,10 @@ static int ospf_te_parse_te(struct ls_ted *ted, struct ospf_lsa *lsa)
}
/* Get corresponding Edge from Link State Data Base */
+ if (IPV4_NET0(attr.standard.local.s_addr) && !attr.standard.local_id) {
+ ote_debug(" |- Found no TE Link local address/ID. Abort!");
+ return -1;
+ }
edge = get_edge(ted, attr.adv, attr.standard.local);
old = edge->attributes;
--
2.35.5

View File

@ -0,0 +1,130 @@
From 2bbcfeb311533ddcebb0d25a9acb4675324ab03f 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 ef45d5c46..236def2da 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1294,6 +1294,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
@@ -2892,8 +2893,6 @@ bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args)
struct attr *const attr = args->attr;
bgp_attr_parse_ret_t ret;
- attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID);
-
uint8_t type;
uint16_t length;
size_t headersz = sizeof(type) + sizeof(length);
@@ -2943,6 +2942,8 @@ bgp_attr_parse_ret_t 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.35.5
From 752612019f22277c387c5711305891d0b713e6c4 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 236def2da..2c4fc70c4 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1285,6 +1285,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
@@ -1318,19 +1327,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.
--
2.35.5

View File

@ -0,0 +1,69 @@
From fc1c932ba7384d69d76b3afe05eb3940ceeb6114 Mon Sep 17 00:00:00 2001
From: Olivier Dugeon <olivier.dugeon@orange.com>
Date: Wed, 3 Apr 2024 16:28:23 +0200
Subject: [PATCH] ospfd: Solved crash in RI parsing with OSPF TE
Iggy Frankovic discovered another ospfd crash when performing fuzzing of OSPF
LSA packets. The crash occurs in ospf_te_parse_ri() function when attemping to
read Segment Routing subTLVs. The original code doesn't check if the size of
the SR subTLVs have the correct length. In presence of erronous LSA, this will
cause a buffer overflow and ospfd crash.
This patch introduces new verification of the subTLVs size for Router
Information TLV.
Co-authored-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
CVE: CVE-2024-31950
Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/f69d1313b19047d3d83fc2b36a518355b861dfc4]
Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
---
ospfd/ospf_te.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 4e420edb3..8247c44a3 100644
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -2492,6 +2492,9 @@ static int ospf_te_parse_ri(struct ls_ted *ted, struct ospf_lsa *lsa)
switch (ntohs(tlvh->type)) {
case RI_SR_TLV_SR_ALGORITHM:
+ if (TLV_BODY_SIZE(tlvh) < 1 ||
+ TLV_BODY_SIZE(tlvh) > ALGORITHM_COUNT)
+ break;
algo = (struct ri_sr_tlv_sr_algorithm *)tlvh;
for (int i = 0; i < ntohs(algo->header.length); i++) {
@@ -2516,6 +2519,8 @@ static int ospf_te_parse_ri(struct ls_ted *ted, struct ospf_lsa *lsa)
break;
case RI_SR_TLV_SRGB_LABEL_RANGE:
+ if (TLV_BODY_SIZE(tlvh) != RI_SR_TLV_LABEL_RANGE_SIZE)
+ break;
range = (struct ri_sr_tlv_sid_label_range *)tlvh;
size = GET_RANGE_SIZE(ntohl(range->size));
lower = GET_LABEL(ntohl(range->lower.value));
@@ -2533,6 +2538,8 @@ static int ospf_te_parse_ri(struct ls_ted *ted, struct ospf_lsa *lsa)
break;
case RI_SR_TLV_SRLB_LABEL_RANGE:
+ if (TLV_BODY_SIZE(tlvh) != RI_SR_TLV_LABEL_RANGE_SIZE)
+ break;
range = (struct ri_sr_tlv_sid_label_range *)tlvh;
size = GET_RANGE_SIZE(ntohl(range->size));
lower = GET_LABEL(ntohl(range->lower.value));
@@ -2550,6 +2557,8 @@ static int ospf_te_parse_ri(struct ls_ted *ted, struct ospf_lsa *lsa)
break;
case RI_SR_TLV_NODE_MSD:
+ if (TLV_BODY_SIZE(tlvh) < RI_SR_TLV_NODE_MSD_SIZE)
+ break;
msd = (struct ri_sr_tlv_node_msd *)tlvh;
if ((CHECK_FLAG(node->flags, LS_NODE_MSD))
&& (node->msd == msd->value))
--
2.35.5

View File

@ -0,0 +1,111 @@
From 8dd8c6343b5aa930b7844a0e481267f3e805d906 Mon Sep 17 00:00:00 2001
From: Olivier Dugeon <olivier.dugeon@orange.com>
Date: Fri, 5 Apr 2024 12:57:11 +0200
Subject: [PATCH] ospfd: Correct Opaque LSA Extended parser
Iggy Frankovic discovered another ospfd crash when performing fuzzing of OSPF
LSA packets. The crash occurs in ospf_te_parse_ext_link() function when
attemping to read Segment Routing Adjacency SID subTLVs. The original code
doesn't check if the size of the Extended Link TLVs and subTLVs have the correct
length. In presence of erronous LSA, this will cause a buffer overflow and ospfd
crashes.
This patch introduces new verification of the subTLVs size for Extended Link
TLVs and subTLVs. Similar check has been also introduced for the Extended
Prefix TLV.
Co-authored-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
CVE: CVE-2024-31951
Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/5557a289acdaeec8cc63ffc97b5c2abf6dee7b3a]
Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
---
ospfd/ospf_te.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 8247c44a3..1404506e5 100644
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -2656,6 +2656,7 @@ static int ospf_te_parse_ext_pref(struct ls_ted *ted, struct ospf_lsa *lsa)
struct ext_tlv_prefix *ext;
struct ext_subtlv_prefix_sid *pref_sid;
uint32_t label;
+ uint16_t len, size;
/* Get corresponding Subnet from Link State Data Base */
ext = (struct ext_tlv_prefix *)TLV_HDR_TOP(lsa->data);
@@ -2677,6 +2678,18 @@ static int ospf_te_parse_ext_pref(struct ls_ted *ted, struct ospf_lsa *lsa)
ote_debug(" |- Process Extended Prefix LSA %pI4 for subnet %pFX",
&lsa->data->id, &pref);
+ /*
+ * Check Extended Prefix TLV size against LSA size
+ * as only one TLV is allowed per LSA
+ */
+ len = TLV_BODY_SIZE(&ext->header);
+ size = lsa->size - (OSPF_LSA_HEADER_SIZE + TLV_HDR_SIZE);
+ if (len != size || len <= 0) {
+ ote_debug(" |- Wrong TLV size: %u instead of %u",
+ (uint32_t)len, (uint32_t)size);
+ return -1;
+ }
+
/* Initialize TLV browsing */
ls_pref = subnet->ls_pref;
pref_sid = (struct ext_subtlv_prefix_sid *)((char *)(ext) + TLV_HDR_SIZE
@@ -2791,8 +2804,20 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
ote_debug(" |- Process Extended Link LSA %pI4 for edge %pI4",
&lsa->data->id, &edge->attributes->standard.local);
- /* Initialize TLV browsing */
- len = TLV_BODY_SIZE(&ext->header) - EXT_TLV_LINK_SIZE;
+ /*
+ * Check Extended Link TLV size against LSA size
+ * as only one TLV is allowed per LSA
+ */
+ len = TLV_BODY_SIZE(&ext->header);
+ i = lsa->size - (OSPF_LSA_HEADER_SIZE + TLV_HDR_SIZE);
+ if (len != i || len <= 0) {
+ ote_debug(" |- Wrong TLV size: %u instead of %u",
+ (uint32_t)len, (uint32_t)i);
+ return -1;
+ }
+
+ /* Initialize subTLVs browsing */
+ len -= EXT_TLV_LINK_SIZE;
tlvh = (struct tlv_header *)((char *)(ext) + TLV_HDR_SIZE
+ EXT_TLV_LINK_SIZE);
for (; sum < len; tlvh = TLV_HDR_NEXT(tlvh)) {
@@ -2802,6 +2827,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
switch (ntohs(tlvh->type)) {
case EXT_SUBTLV_ADJ_SID:
+ if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_ADJ_SID_SIZE)
+ break;
adj = (struct ext_subtlv_adj_sid *)tlvh;
label = CHECK_FLAG(adj->flags,
EXT_SUBTLV_LINK_ADJ_SID_VFLG)
@@ -2828,6 +2855,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
break;
case EXT_SUBTLV_LAN_ADJ_SID:
+ if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_LAN_ADJ_SID_SIZE)
+ break;
ladj = (struct ext_subtlv_lan_adj_sid *)tlvh;
label = CHECK_FLAG(ladj->flags,
EXT_SUBTLV_LINK_ADJ_SID_VFLG)
@@ -2857,6 +2886,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
break;
case EXT_SUBTLV_RMT_ITF_ADDR:
+ if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_RMT_ITF_ADDR_SIZE)
+ break;
rmt = (struct ext_subtlv_rmt_itf_addr *)tlvh;
if (CHECK_FLAG(atr->flags, LS_ATTR_NEIGH_ADDR)
&& IPV4_ADDR_SAME(&atr->standard.remote,
--
2.35.5

View File

@ -0,0 +1,84 @@
From 10ff8433557df40c6d7e4361cc468a1192185fdd Mon Sep 17 00:00:00 2001
From: Olivier Dugeon <olivier.dugeon@orange.com>
Date: Tue, 16 Apr 2024 16:42:06 +0200
Subject: [PATCH] ospfd: protect call to get_edge() in ospf_te.c
During fuzzing, Iggy Frankovic discovered that get_edge() function in ospf_te.c
could return null pointer, in particular when the link_id or advertised router
IP addresses are fuzzed. As the null pointer returned by get_edge() function is
not handlei by calling functions, this could cause ospfd crash.
This patch introduces new verification of returned pointer by get_edge()
function and stop the processing in case of null pointer. In addition, link ID
and advertiser router ID are validated before calling ls_find_edge_by_key() to
avoid the creation of a new edge with an invalid key.
CVE-2024-34088
Co-authored-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
CVE: CVE-2024-34088
Upstream-Status: Backport [https://github.com/FRRouting/frr/commit/8c177d69e32b91b45bda5fc5da6511fa03dc11ca]
Signed-off-by: Zhang Peng <peng.zhang1.cn@windriver.com>
---
ospfd/ospf_te.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 5af006e54..4e420edb3 100644
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -1686,6 +1686,11 @@ static struct ls_edge *get_edge(struct ls_ted *ted, struct ls_node_id adv,
struct ls_edge *edge;
struct ls_attributes *attr;
+ /* Check that Link ID and Node ID are valid */
+ if (IPV4_NET0(link_id.s_addr) || IPV4_NET0(adv.id.ip.addr.s_addr) ||
+ adv.origin != OSPFv2)
+ return NULL;
+
/* Search Edge that corresponds to the Link ID */
key = ((uint64_t)ntohl(link_id.s_addr)) & 0xffffffff;
edge = ls_find_edge_by_key(ted, key);
@@ -1758,6 +1763,10 @@ static void ospf_te_update_link(struct ls_ted *ted, struct ls_vertex *vertex,
/* Get Corresponding Edge from Link State Data Base */
edge = get_edge(ted, vertex->node->adv, link_data);
+ if (!edge) {
+ ote_debug(" |- Found no edge from Link Data. Abort!");
+ return;
+ }
attr = edge->attributes;
/* re-attached edge to vertex if needed */
@@ -2276,11 +2285,11 @@ static int ospf_te_parse_te(struct ls_ted *ted, struct ospf_lsa *lsa)
}
/* Get corresponding Edge from Link State Data Base */
- if (IPV4_NET0(attr.standard.local.s_addr) && !attr.standard.local_id) {
- ote_debug(" |- Found no TE Link local address/ID. Abort!");
+ edge = get_edge(ted, attr.adv, attr.standard.local);
+ if (!edge) {
+ ote_debug(" |- Found no edge from Link local add./ID. Abort!");
return -1;
}
- edge = get_edge(ted, attr.adv, attr.standard.local);
old = edge->attributes;
ote_debug(" |- Process Traffic Engineering LSA %pI4 for Edge %pI4",
@@ -2764,6 +2773,10 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
lnid.id.ip.area_id = lsa->area->area_id;
ext = (struct ext_tlv_link *)TLV_HDR_TOP(lsa->data);
edge = get_edge(ted, lnid, ext->link_data);
+ if (!edge) {
+ ote_debug(" |- Found no edge from Extended Link Data. Abort!");
+ return -1;
+ }
atr = edge->attributes;
ote_debug(" |- Process Extended Link LSA %pI4 for edge %pI4",
--
2.35.5

View File

@ -28,7 +28,12 @@ SRC_URI = "git://github.com/FRRouting/frr.git;protocol=https;branch=stable/8.2 \
file://CVE-2023-47234.patch \
file://CVE-2023-47235.patch \
file://frr.pam \
file://CVE-2024-44070.patch\
file://CVE-2024-44070.patch \
file://CVE-2024-27913.patch \
file://CVE-2024-34088.patch \
file://CVE-2024-31950.patch \
file://CVE-2024-31951.patch \
file://CVE-2024-31948.patch \
"
SRCREV = "79188bf710e92acf42fb5b9b0a2e9593a5ee9b05"