The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=9ba51cce8bbd8bb2bb761b1698a875c81dee317f
commit 9ba51cce8bbd8bb2bb761b1698a875c81dee317f Author: Gleb Smirnoff <gleb...@freebsd.org> AuthorDate: 2025-07-25 20:10:53 +0000 Commit: Gleb Smirnoff <gleb...@freebsd.org> CommitDate: 2025-07-25 20:10:53 +0000 libbsnmp: make snmp_parse_server() more robust when lacking info Per documentation snmp_parse_server() has all 4 components optional. We want inputs like "udp::", "udp6::", "stream::" work with local default sockets and even degenerate case of "" shall end with defaults instead of a failure. While here make the parser more robust. Make all parser helpers to behave the same way: on successful return they are responsible to update both start & end of matched substring and return next characater to parse. This eliminates special handling of IPv6 and port parser that used to need to skip a character, and thus eliminates unchecked step forward in snmp_parse_server() itself. Additionally: - use strchr(3) instead of own cycle - INET_ADDRSTRLEN includes space for NUL character - INET6_ADDRSTRLEN includes both the scope and NUL character - prefer sizeof() comparison instead of preprocessor define repetitive use Reviewed by: harti Differential Revision: https://reviews.freebsd.org/D51072 --- contrib/bsnmp/lib/snmpclient.c | 183 +++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 98 deletions(-) diff --git a/contrib/bsnmp/lib/snmpclient.c b/contrib/bsnmp/lib/snmpclient.c index 5fa7d4f5be2a..44dfbd5a06b1 100644 --- a/contrib/bsnmp/lib/snmpclient.c +++ b/contrib/bsnmp/lib/snmpclient.c @@ -1942,70 +1942,64 @@ get_transp(struct snmp_client *sc, const char **strp) * community strings are legal. * * \param sc client struct to set errors - * \param strp possible start of community; updated to the point to - * the next character to parse + * \param comm possible start of community; updated to start & end * - * \return end of community; equals *strp if there is none; NULL if there - * was an error + * \return the next character to parse; NULL if there was an error */ static inline const char * -get_comm(struct snmp_client *sc, const char **strp) +get_comm(struct snmp_client *sc, const char *comm[2]) { - const char *p = strrchr(*strp, '@'); + const char *p = strrchr(comm[0], '@'); if (p == NULL) /* no community string */ - return (*strp); + return (comm[1] = comm[0]); - if (p - *strp > SNMP_COMMUNITY_MAXLEN) { + if (p - comm[0] > SNMP_COMMUNITY_MAXLEN) { seterr(sc, "community string too long '%.*s'", - p - *strp, *strp); + p - comm[0], comm[0]); return (NULL); } - *strp = p + 1; - return (p); + return ((comm[1] = p) + 1); } /** * Try to get an IPv6 address. This starts with an [ and should end with an ] * and everything between should be not longer than INET6_ADDRSTRLEN and - * parseable by inet_pton(). + * parseable by getaddrinfo(). * * \param sc client struct to set errors - * \param strp possible start of IPv6 address (the '['); updated to point to - * the next character to parse (the one after the closing ']') + * \param ipv6 possible start of IPv6 address (the '['); updated to actual + * start (one after '[') and actual end (the '[' itself) * - * \return end of address (equals *strp + 1 if there is none) or NULL - * on errors + * \return the next character to parse (the one after the closing ']') + * or NULL on errors */ static inline const char * -get_ipv6(struct snmp_client *sc, const char **strp) +get_ipv6(struct snmp_client *sc, const char *ipv6[2]) { - char str[INET6_ADDRSTRLEN + IF_NAMESIZE]; + char str[INET6_ADDRSTRLEN]; + const char *p; struct addrinfo hints, *res; int error; - if (**strp != '[') - return (*strp + 1); + if (ipv6[0][0] != '[') + return (ipv6[1] = ipv6[0]); - const char *p = *strp + 1; - while (*p != ']' ) { - if (*p == '\0') { - seterr(sc, "unterminated IPv6 address '%.*s'", - p - *strp, *strp); - return (NULL); - } - p++; + if ((p = strchr(++(ipv6[0]), ']')) == NULL) { + seterr(sc, "unterminated IPv6 address '%s'", ipv6[0]); + return (NULL); } - if (p - *strp > INET6_ADDRSTRLEN + IF_NAMESIZE) { - seterr(sc, "IPv6 address too long '%.*s'", p - *strp, *strp); + if ((size_t)(p - ipv6[0]) >= sizeof(str)) { + seterr(sc, "IPv6 address too long '%.*s'", + p - ipv6[0], ipv6[0]); return (NULL); } - strncpy(str, *strp + 1, p - (*strp + 1)); - str[p - (*strp + 1)] = '\0'; + strncpy(str, ipv6[0], p - ipv6[0]); + str[p - ipv6[0]] = '\0'; memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_CANONNAME | AI_NUMERICHOST; @@ -2018,8 +2012,7 @@ get_ipv6(struct snmp_client *sc, const char **strp) return (NULL); } freeaddrinfo(res); - *strp = p + 1; - return (p); + return ((ipv6[1] = p) + 1); } /** @@ -2028,30 +2021,29 @@ get_ipv6(struct snmp_client *sc, const char **strp) * inet_aton(). * * \param sc client struct to set errors - * \param strp possible start of IPv4 address; updated to point to the - * next character to parse + * \param ipv4 possible start of IPv4 address; updated to start & end * - * \return end of address (equals *strp if there is none) or NULL - * on errors + * \return the next character to parse; or NULL on errors */ static inline const char * -get_ipv4(struct snmp_client *sc, const char **strp) +get_ipv4(struct snmp_client *sc, const char *ipv4[2]) { - const char *p = *strp; + char str[INET_ADDRSTRLEN]; + const char *p = ipv4[0]; while (isascii(*p) && (isdigit(*p) || *p == '.')) p++; - if (p - *strp > INET_ADDRSTRLEN) { - seterr(sc, "IPv4 address too long '%.*s'", p - *strp, *strp); + if ((size_t)(p - ipv4[0]) >= sizeof(str)) { + seterr(sc, "IPv4 address too long '%.*s'", + p - ipv4[0], ipv4[0]); return (NULL); } - if (*strp == p) - return *strp; + if (p == ipv4[0]) + return (ipv4[1] = ipv4[0]); - char str[INET_ADDRSTRLEN + 1]; - strncpy(str, *strp, p - *strp); - str[p - *strp] = '\0'; + strncpy(str, ipv4[0], p - ipv4[0]); + str[p - ipv4[0]] = '\0'; struct in_addr addr; if (inet_aton(str, &addr) != 1) { @@ -2059,8 +2051,7 @@ get_ipv4(struct snmp_client *sc, const char **strp) return (NULL); } - *strp = p; - return (p); + return (ipv4[1] = p); } /** @@ -2068,24 +2059,19 @@ get_ipv4(struct snmp_client *sc, const char **strp) * the last colon (if any). There is no length restriction. * * \param sc client struct to set errors - * \param strp possible start of hostname; updated to point to the next - * character to parse (the trailing NUL character or the last - * colon) + * \param host possible start of hostname; start & end updated * - * \return end of address (equals *strp if there is none) + * \return next character to parse (semicolon or NUL) */ static inline const char * -get_host(struct snmp_client *sc __unused, const char **strp) +get_host(struct snmp_client *sc __unused, const char *host[2]) { - const char *p = strrchr(*strp, ':'); + const char *p = strrchr(host[0], ':'); - if (p == NULL) { - *strp += strlen(*strp); - return (*strp); - } + if (p == NULL) + return (host[1] = host[0] + strlen(host[0])); - *strp = p; - return (p); + return (host[1] = p); } /** @@ -2093,25 +2079,24 @@ get_host(struct snmp_client *sc __unused, const char **strp) * of string. The port number must not be empty. * * \param sc client struct to set errors - * \param strp possible start of port specification; if this points to a + * \param port possible start of port specification; if this points to a * colon there is a port specification * * \return end of port number (equals *strp if there is none); NULL * if there is no port number */ static inline const char * -get_port(struct snmp_client *sc, const char **strp) +get_port(struct snmp_client *sc, const char *port[2]) { - if (**strp != ':') - return (*strp + 1); + if (*port[0] != ':') + return (port[1] = port[0]); - if ((*strp)[1] == '\0') { + if (port[0][1] == '\0') { seterr(sc, "empty port name"); return (NULL); } - *strp += strlen(*strp); - return (*strp); + return (port[1] = ++(port[0]) + strlen(port[0])); } /** @@ -2172,6 +2157,7 @@ int snmp_parse_server(struct snmp_client *sc, const char *str) { const char *const orig = str; + const char *comm[2], *ipv6[2], *ipv4[2], *host[2], *port[2]; /* parse input */ int def_trans = 0, trans = get_transp(sc, &str); @@ -2181,42 +2167,32 @@ snmp_parse_server(struct snmp_client *sc, const char *str) if (orig == str) def_trans = 1; - const char *const comm[2] = { - str, - get_comm(sc, &str), - }; - if (comm[1] == NULL) + comm[0] = str; + if ((str = get_comm(sc, comm)) == NULL) return (-1); - const char *const ipv6[2] = { - str + 1, - get_ipv6(sc, &str), - }; - if (ipv6[1] == NULL) + ipv6[0] = str; + if ((str = get_ipv6(sc, ipv6)) == NULL) return (-1); - const char *ipv4[2] = { - str, - str, - }; - - const char *host[2] = { - str, - str, - }; - if (ipv6[0] == ipv6[1]) { - ipv4[1] = get_ipv4(sc, &str); + ipv4[0] = str; + if ((str = get_ipv4(sc, ipv4)) == NULL) { + /* This failure isn't fatal: restore str. */ + str = ipv4[0]; + ipv4[0] = ipv4[1] = NULL; + } - if (ipv4[0] == ipv4[1]) - host[1] = get_host(sc, &str); - } + if (ipv4[0] == ipv4[1]) { + host[0] = str; + str = get_host(sc, host); + } else + host[0] = host[1] = NULL; + } else + ipv4[0] = ipv4[1] = host[0] = host[1] = NULL; - const char *port[2] = { - str + 1, - get_port(sc, &str), - }; - if (port[1] == NULL) + port[0] = str; + if ((str = get_port(sc, port)) == NULL) return (-1); if (*str != '\0') { @@ -2247,7 +2223,7 @@ snmp_parse_server(struct snmp_client *sc, const char *str) return (-1); if (def_trans) trans = SNMP_TRANS_UDP; - } else { + } else if (host[0] != host[1]) { if ((chost = save_str(sc, host)) == NULL) return (-1); @@ -2262,6 +2238,17 @@ snmp_parse_server(struct snmp_client *sc, const char *str) break; } } + } else switch (trans) { + case SNMP_TRANS_UDP: + case SNMP_TRANS_UDP6: + if ((chost = strdup(DEFAULT_HOST)) == NULL) + return (-1); + break; + case SNMP_TRANS_LOC_DGRAM: + case SNMP_TRANS_LOC_STREAM: + if ((chost = strdup(SNMP_DEFAULT_LOCAL)) == NULL) + return (-1); + break; } char *cport;