From 34fdeb0ed34147933125d1bb4f8ab90c1f2a7ca0 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Fri, 31 Mar 2023 17:46:21 -0400 Subject: [PATCH] bgpd: fix ecommunity parsing for AS4 The parser for extended communities was incorrectly disallowing an operator from configuring "Route Origin" extended communities (e.g. RD/RT/SoO) with a 4-byte value matching BGP_AS4_MAX (UINT32_MAX) and allowed the user to overflow UINT32_MAX. This updates the parser to read the value as a uint64_t so that we can do proper checks on the upper bounds (> BGP_AS4_MAX || errno). before: ``` TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65 TORC11(config-router-af)# do sh run | include soo neighbor uplink-1 soo 0:65 TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65 % Malformed SoO extended community TORC11(config-router-af)# ``` after: ``` TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65 % Malformed SoO extended community TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65 TORC11(config-router-af)# do sh run | include soo neighbor uplink-1 soo 4294967295:65 TORC11(config-router-af)# ``` Signed-off-by: Trey Aspelund (cherry picked from commit b571d79d6482217c599c53976cb8cf0ec77b847f) --- bgpd/bgp_ecommunity.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c index 589d9af1e5..2599bf258d 100644 --- a/bgpd/bgp_ecommunity.c +++ b/bgpd/bgp_ecommunity.c @@ -520,6 +520,8 @@ static const char *ecommunity_gettoken(const char *str, uint8_t ecomm_type; char buf[INET_ADDRSTRLEN + 1]; struct ecommunity_val *eval = (struct ecommunity_val *)eval_ptr; + uint64_t tmp_as = 0; + /* Skip white space. */ while (isspace((unsigned char)*p)) { p++; @@ -598,9 +600,18 @@ static const char *ecommunity_gettoken(const char *str, goto error; endptr++; - as = strtoul(endptr, &endptr, 10); - if (*endptr != '\0' || as == BGP_AS4_MAX) + errno = 0; + tmp_as = strtoul(endptr, &endptr, 10); + /* 'unsigned long' is a uint64 on 64-bit + * systems, and uint32 on 32-bit systems. So for + * 64-bit we can just directly check the value + * against BGP_AS4_MAX/UINT32_MAX, and for + * 32-bit we can check for errno (set to ERANGE + * upon overflow). + */ + if (*endptr != '\0' || tmp_as == BGP_AS4_MAX || errno) goto error; + as = (as_t)tmp_as; memcpy(buf, p, (limit - p)); buf[limit - p] = '\0'; @@ -642,9 +653,19 @@ static const char *ecommunity_gettoken(const char *str, goto error; } else { /* ASN */ - as = strtoul(buf, &endptr, 10); - if (*endptr != '\0' || as == BGP_AS4_MAX) + errno = 0; + tmp_as = strtoul(buf, &endptr, 10); + /* 'unsigned long' is a uint64 on 64-bit + * systems, and uint32 on 32-bit systems. So for + * 64-bit we can just directly check the value + * against BGP_AS4_MAX/UINT32_MAX, and for + * 32-bit we can check for errno (set to ERANGE + * upon overflow). + */ + if (*endptr != '\0' || tmp_as > BGP_AS4_MAX || + errno) goto error; + as = (as_t)tmp_as; } } else if (*p == '.') { if (separator) -- 2.39.5