Merge pull request #14511 from opensourcerouting/fix/bgpd_software_ve…

…rsion_capability bgpd: Validate maximum length of software version when handling via dynamic caps
FRRouting · Oct 3, 2023 · 373d46d · 373d46d
2 parents c97c31c + 0e43f80
commit 373d46d
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 18 deletions.
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
@@ -3064,6 +3064,40 @@ static void bgp_dynamic_capability_graceful_restart(uint8_t *pnt, int action,
 	}
 }
 
+static void bgp_dynamic_capability_software_version(uint8_t *pnt, int action,
+						    struct capability_header *hdr,
+						    struct peer *peer)
+{
+	uint8_t *data = pnt + 3;
+	uint8_t *end = data + hdr->length;
+	uint8_t len = *data;
+	char soft_version[BGP_MAX_SOFT_VERSION + 1] = {};
+
+	if (action == CAPABILITY_ACTION_SET) {
+		if (data + len > end) {
+			zlog_err("%pBP: Received invalid Software Version capability length %d",
+				 peer, len);
+			return;
+		}
+		data++;
+
+		if (len > BGP_MAX_SOFT_VERSION)
+			len = BGP_MAX_SOFT_VERSION;
+
+		memcpy(&soft_version, data, len);
+		soft_version[len] = '\0';
+
+		XFREE(MTYPE_BGP_SOFT_VERSION, peer->soft_version);
+		peer->soft_version = XSTRDUP(MTYPE_BGP_SOFT_VERSION,
+					     soft_version);
+
+		SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV);
+	} else {
+		UNSET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV);
+		XFREE(MTYPE_BGP_SOFT_VERSION, peer->soft_version);
+	}
+}
+
 /**
  * Parse BGP CAPABILITY message for peer.
  *
@@ -3082,7 +3116,6 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
 	afi_t afi;
 	iana_safi_t pkt_safi;
 	safi_t safi;
-	char soft_version[BGP_MAX_SOFT_VERSION + 1] = {};
 	const char *capability;
 
 	end = pnt + length;
@@ -3129,22 +3162,8 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
 
 		switch (hdr->code) {
 		case CAPABILITY_CODE_SOFT_VERSION:
-			if (action == CAPABILITY_ACTION_SET) {
-				SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV);
-
-				memcpy(&soft_version, pnt + 3, hdr->length);
-				soft_version[hdr->length] = '\0';
-
-				XFREE(MTYPE_BGP_SOFT_VERSION,
-				      peer->soft_version);
-				peer->soft_version =
-					XSTRDUP(MTYPE_BGP_SOFT_VERSION,
-						soft_version);
-			} else {
-				UNSET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV);
-				XFREE(MTYPE_BGP_SOFT_VERSION,
-				      peer->soft_version);
-			}
+			bgp_dynamic_capability_software_version(pnt, action,
+								hdr, peer);
 			break;
 		case CAPABILITY_CODE_MP:
 			if (hdr->length < sizeof(struct capability_mp_data)) {

diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py
@@ -111,7 +111,7 @@ def _bgp_software_version():
                 if not adv and not rcv:
                     return ""
 
-                pattern = "FRRouting/\\d.+"
+                pattern = "^FRRouting/\\d.+"
                 if re.search(pattern, adv) and re.search(pattern, rcv):
                     return adv, rcv
             except: