From 5e8a8d0ed63011ca83a20cf5c07b53f0cce78d4b Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 29 Sep 2023 12:13:43 +0300 Subject: bgpd: Validate maximum length of software version when handling via dynamic caps We should not allow exceeding the stream's length, and also software version can't be larger than 64 bytes. Signed-off-by: Donatas Abraitis --- bgpd/bgp_packet.c | 53 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index d47324593..5d7441ed6 100644 --- 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)) { -- cgit v1.2.3 From 0e43f80ab921633d786cd9f6e9823acb6a3a58d1 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 29 Sep 2023 12:15:24 +0300 Subject: tests: Make sure we have a valid FRRouting software version string It can't begin with anything else, otherwise something is broken on the wire. Signed-off-by: Donatas Abraitis --- .../test_bgp_dynamic_capability_software_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index d1069a876..a653da465 100644 --- 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 test_bgp_dynamic_capability_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: -- cgit v1.2.3