git: 9ba51cce8bbd - main - libbsnmp: make snmp_parse_server() more robust when lacking info
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 25 Jul 2025 20:12:06 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=9ba51cce8bbd8bb2bb761b1698a875c81dee317f
commit 9ba51cce8bbd8bb2bb761b1698a875c81dee317f
Author: Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2025-07-25 20:10:53 +0000
Commit: Gleb Smirnoff <glebius@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;