Merge pull request #22056 from opensourcerouting/fix/bgp_issue_22043

bgpd: Fix stack overflow when debug printing label information & BMP code

Author: Jafaral

bgpd/bgp_bmp.c

@@ -1106,7 +1106,15 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 
 	bpacket_attr_vec_arr_reset(&vecarr);
 
-	s = stream_new(BGP_MAX_PACKET_SIZE);
+	/*
+	 * Use an expandable stream so that an oversized re-encoded UPDATE
+	 * (large unknown-transitive attribute plus route-map additions) does
+	 * not silently truncate via CHECK_SIZE or assert in stream_put_prefix.
+	 * The on-wire UPDATE inside a BMP Route-Monitoring message must still
+	 * fit BGP's 16-bit length field, so we drop the route if the encoded
+	 * size exceeds BGP_MAX_PACKET_SIZE.
+	 */
+	s = stream_new_expandable(BGP_MAX_PACKET_SIZE);
 	bgp_packet_set_marker(s, BGP_MSG_UPDATE);
 
 	/* 2: withdrawn routes length */
@@ -1120,8 +1128,6 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 	total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi, safi, peer,
 					      NULL, NULL, 0, 0, 0, 0, NULL, NULL);
 
-	/* space check? */
-
 	/* peer_cap_enhe & add-path removed */
 	if (afi == AFI_IP && safi == SAFI_UNICAST)
 		stream_put_prefix(s, p);
@@ -1137,6 +1143,13 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 		total_attr_len += stream_get_endp(s) - p1;
 	}
 
+	if (stream_get_endp(s) > BGP_MAX_PACKET_SIZE) {
+		zlog_warn("bmp: skipping route-monitoring for %pFX: re-encoded UPDATE exceeds %u bytes",
+			  p, BGP_MAX_PACKET_SIZE);
+		stream_free(s);
+		return NULL;
+	}
+
 	/* set the total attribute length correctly */
 	stream_putw_at(s, attrlen_pos, total_attr_len);
 	bgp_packet_set_size(s);
@@ -1152,7 +1165,7 @@ static struct stream *bmp_withdraw(const struct prefix *p,
 	bgp_size_t total_attr_len = 0;
 	bgp_size_t unfeasible_len;
 
-	s = stream_new(BGP_MAX_PACKET_SIZE);
+	s = stream_new_expandable(BGP_MAX_PACKET_SIZE);
 
 	bgp_packet_set_marker(s, BGP_MSG_UPDATE);
 	stream_putw(s, 0);
@@ -1179,6 +1192,13 @@ static struct stream *bmp_withdraw(const struct prefix *p,
 		stream_putw_at(s, attrlen_pos, total_attr_len);
 	}
 
+	if (stream_get_endp(s) > BGP_MAX_PACKET_SIZE) {
+		zlog_warn("bmp: skipping route-monitoring withdraw for %pFX: encoded UPDATE exceeds %u bytes",
+			  p, BGP_MAX_PACKET_SIZE);
+		stream_free(s);
+		return NULL;
+	}
+
 	bgp_packet_set_size(s);
 	return s;
 }
@@ -1208,6 +1228,9 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags,
 	else
 		msg = bmp_withdraw(p, prd, afi, safi);
 
+	if (!msg)
+		return;
+
 	hdr = stream_new(BGP_MAX_PACKET_SIZE);
 	bmp_common_hdr(hdr, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING);
 	bmp_per_peer_hdr(hdr, peer->bgp, peer, flags, peer_type_flag, peer_distinguisher,

bgpd/bgp_debug.c

@@ -2951,7 +2951,7 @@ const char *bgp_debug_rdpfxpath2str(afi_t afi, safi_t safi,
 				    struct bgp_route_evpn *overlay_index,
 				    char *str, int size)
 {
-	char tag_buf[30];
+	char tag_buf[sizeof(" label ") + BGP_MAX_LABEL_DIGITS];
 	char overlay_index_buf[INET6_ADDRSTRLEN + 14];
 	const struct prefix_evpn *evp;
 	int len = 0;

bgpd/bgp_label.c

@@ -681,24 +681,30 @@ char *mpls_labels2str(mpls_label_t *labels, uint8_t num_labels, const char *pref
 		      int size)
 {
 	uint32_t label_value;
-	uint32_t len = 0;
+	int len = 0;
+	int ret;
 	uint8_t i;
 
+	if (size <= 0)
+		return buf;
+
 	buf[0] = '\0';
 	if (!num_labels)
 		return buf;
 
 	if (prefix) {
-		strlcat(buf + len, prefix, size - len);
+		strlcat(buf, prefix, size);
 		len = strlen(buf);
+		if (len >= size - 1)
+			return buf;
 	}
 
-	label_value = decode_label(&labels[0]);
-	len += snprintf(buf + len, size - len, "%u", label_value);
-
-	for (i = 1; i < num_labels; i++) {
+	for (i = 0; i < num_labels; i++) {
 		label_value = decode_label(&labels[i]);
-		len += snprintf(buf + len, size - len, "/%u", label_value);
+		ret = snprintf(buf + len, size - len, "%s%u", i == 0 ? "" : "/", label_value);
+		if (ret < 0 || ret >= size - len)
+			return buf;
+		len += ret;
 	}
 
 	return buf;

bgpd/bgp_label.h

@@ -23,6 +23,7 @@ struct peer;
  * It is impossible to pass more than 10 labels by BGP-LU
  */
 #define BGP_MAX_LABELS 10
+#define BGP_MAX_LABEL_DIGITS (9 * BGP_MAX_LABELS)
 
 /* MPLS label(s) - VNI(s) for EVPN-VxLAN  */
 struct bgp_labels {

bgpd/bgp_route.c

@@ -12436,7 +12436,7 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
 			  struct attr *pattr, uint16_t show_opts)
 {
 	char buf[INET6_ADDRSTRLEN];
-	char labels_buf[9 * BGP_MAX_LABELS]; /* 8 per label + / or \0 for each */
+	char labels_buf[BGP_MAX_LABEL_DIGITS]; /* 8 per label + / or \0 for each */
 	char vni_buf[30] = {};
 	struct attr *attr = pattr ? pattr : path->attr;
 	time_t tbuf;

bgpd/bgp_zebra.c

@@ -1545,7 +1545,7 @@ static void bgp_debug_zebra_nh(struct zapi_route *api)
 	char eth_buf[ETHER_ADDR_STRLEN + 7] = { '\0' };
 	char buf1[ETHER_ADDR_STRLEN];
 	/* strlen("label ") + 8 chars per label + '/' or '\0' for each label */
-	char label_buf[6 + 9 * BGP_MAX_LABELS];
+	char label_buf[6 + BGP_MAX_LABEL_DIGITS];
 	char sid_buf[20];
 	char segs_buf[256];
 	struct zapi_nexthop *api_nh;