frr: Security fix CVE-2022-37035

CVE-2022-37035:
An issue was discovered in bgpd in FRRouting (FRR) 8.3. In
bgp_notify_send_with_data() and bgp_process_packet() in bgp_packet.c,
there is a possible use-after-free due to a race condition. This could
lead to Remote Code Execution or Information Disclosure by sending
crafted BGP packets. User interaction is not needed for exploitation.

Reference:
https://nvd.nist.gov/vuln/detail/CVE-2022-37035

Patch from:
https://github.com/FRRouting/frr/commit/71ca5b09bc71e8cbe38177cf41e83fe164e52eee

Signed-off-by: Yi Zhao <yi.zhao@windriver.com>
This commit is contained in:
Yi Zhao
2022-09-24 15:39:57 +08:00
committed by Armin Kuster
parent c5b5f631fc
commit fc9c8a3332
2 changed files with 152 additions and 0 deletions
@@ -0,0 +1,151 @@
From db24300d56ad5831d9f6e4545ff2999b99e71bac Mon Sep 17 00:00:00 2001
From: Mark Stapp <mstapp@nvidia.com>
Date: Thu, 8 Sep 2022 16:14:36 -0400
Subject: [PATCH] bgpd: avoid notify race between io and main pthreads
The "bgp_notify_" apis in bgp_packet.c generate a notification
to a peer, usually during error handling. The io pthread wants
to send notifications in a couple of cases during early
received-packet validation - but the existing api interacts
with the peer struct itself, and that's not safe.
Add a new api for use by the io pthread, and adjust the main
notify api so that it can avoid touching the peer struct.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
CVE: CVE-2022-37035
Upstream-Status: Backport
[https://github.com/FRRouting/frr/commit/71ca5b09bc71e8cbe38177cf41e83fe164e52eee]
Signed-off-by: Yi Zhao <yi.zhao@windriver.com>
---
bgpd/bgp_io.c | 17 ++++++++---------
bgpd/bgp_packet.c | 32 ++++++++++++++++++++++++++++----
bgpd/bgp_packet.h | 2 ++
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c
index 9b5a31f28..c736d02db 100644
--- a/bgpd/bgp_io.c
+++ b/bgpd/bgp_io.c
@@ -37,7 +37,7 @@
#include "bgpd/bgp_debug.h" // for bgp_debug_neighbor_events, bgp_type_str
#include "bgpd/bgp_errors.h" // for expanded error reference information
#include "bgpd/bgp_fsm.h" // for BGP_EVENT_ADD, bgp_event
-#include "bgpd/bgp_packet.h" // for bgp_notify_send_with_data, bgp_notify...
+#include "bgpd/bgp_packet.h" // for bgp_notify_io_invalid...
#include "bgpd/bgp_trace.h" // for frrtraces
#include "bgpd/bgpd.h" // for peer, BGP_MARKER_SIZE, bgp_master, bm
/* clang-format on */
@@ -526,8 +526,8 @@ static bool validate_header(struct peer *peer)
return false;
if (memcmp(m_correct, m_rx, BGP_MARKER_SIZE) != 0) {
- bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR,
- BGP_NOTIFY_HEADER_NOT_SYNC);
+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
+ BGP_NOTIFY_HEADER_NOT_SYNC, NULL, 0);
return false;
}
@@ -547,9 +547,8 @@ static bool validate_header(struct peer *peer)
zlog_debug("%s unknown message type 0x%02x", peer->host,
type);
- bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR,
- BGP_NOTIFY_HEADER_BAD_MESTYPE, &type,
- 1);
+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
+ BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, 1);
return false;
}
@@ -574,9 +573,9 @@ static bool validate_header(struct peer *peer)
uint16_t nsize = htons(size);
- bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR,
- BGP_NOTIFY_HEADER_BAD_MESLEN,
- (unsigned char *)&nsize, 2);
+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
+ BGP_NOTIFY_HEADER_BAD_MESLEN,
+ (unsigned char *)&nsize, 2);
return false;
}
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 7c92a8d9e..a5ce5a527 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -736,8 +736,9 @@ static void bgp_write_notify(struct peer *peer)
* @param data Data portion
* @param datalen length of data portion
*/
-void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
- uint8_t sub_code, uint8_t *data, size_t datalen)
+static void bgp_notify_send_internal(struct peer *peer, uint8_t code,
+ uint8_t sub_code, uint8_t *data,
+ size_t datalen, bool use_curr)
{
struct stream *s;
@@ -769,8 +770,11 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
* If possible, store last packet for debugging purposes. This check is
* in place because we are sometimes called with a doppelganger peer,
* who tends to have a plethora of fields nulled out.
+ *
+ * Some callers should not attempt this - the io pthread for example
+ * should not touch internals of the peer struct.
*/
- if (peer->curr) {
+ if (use_curr && peer->curr) {
size_t packetsize = stream_get_endp(peer->curr);
assert(packetsize <= peer->max_packet_size);
memcpy(peer->last_reset_cause, peer->curr->data, packetsize);
@@ -853,7 +857,27 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
*/
void bgp_notify_send(struct peer *peer, uint8_t code, uint8_t sub_code)
{
- bgp_notify_send_with_data(peer, code, sub_code, NULL, 0);
+ bgp_notify_send_internal(peer, code, sub_code, NULL, 0, true);
+}
+
+/*
+ * Enqueue notification; called from the main pthread, peer object access is ok.
+ */
+void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
+ uint8_t sub_code, uint8_t *data, size_t datalen)
+{
+ bgp_notify_send_internal(peer, code, sub_code, data, datalen, true);
+}
+
+/*
+ * For use by the io pthread, queueing a notification but avoiding access to
+ * the peer object.
+ */
+void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code,
+ uint8_t *data, size_t datalen)
+{
+ /* Avoid touching the peer object */
+ bgp_notify_send_internal(peer, code, sub_code, data, datalen, false);
}
/*
diff --git a/bgpd/bgp_packet.h b/bgpd/bgp_packet.h
index 280d3ec17..898f88ff5 100644
--- a/bgpd/bgp_packet.h
+++ b/bgpd/bgp_packet.h
@@ -62,6 +62,8 @@ extern void bgp_open_send(struct peer *);
extern void bgp_notify_send(struct peer *, uint8_t, uint8_t);
extern void bgp_notify_send_with_data(struct peer *, uint8_t, uint8_t,
uint8_t *, size_t);
+void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code,
+ uint8_t *data, size_t datalen);
extern void bgp_route_refresh_send(struct peer *peer, afi_t afi, safi_t safi,
uint8_t orf_type, uint8_t when_to_refresh,
int remove, uint8_t subtype);
--
2.25.1
@@ -10,6 +10,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263 \
file://COPYING-LGPLv2.1;md5=4fbd65380cdd255951079008b364516c"
SRC_URI = "git://github.com/FRRouting/frr.git;protocol=https;branch=stable/8.2 \
file://CVE-2022-37035.patch \
file://frr.pam \
"