From b2437de6174db827a12fe175e0092833bcfd5a2f Mon Sep 17 00:00:00 2001 From: Brad House Date: Sun, 15 Oct 2023 16:28:04 -0400 Subject: [PATCH] SonarCloud: Fix up codesmells due to strlen(), strcpy(), and strncpy() (#576) Create ares_strlen() and ares_strcpy() in order to resolve SonarCloud codesmells related to their use. ares_strlen() just becomes null-safe. ares_strcpy() is equivalent to strlcpy(), so unlike strncpy() it guarantees NULL termination. Fix By: Brad House (@bradh352) --- src/lib/Makefile.inc | 4 +- src/lib/ares__read_line.c | 2 +- src/lib/ares_android.c | 5 +- src/lib/ares_create_query.c | 2 +- src/lib/ares_expand_string.c | 2 +- src/lib/ares_getaddrinfo.c | 6 +-- src/lib/ares_gethostbyaddr.c | 2 +- src/lib/ares_getnameinfo.c | 10 ++-- src/lib/ares_init.c | 18 ++++---- src/lib/ares_options.c | 4 +- src/lib/ares_parse_ns_reply.c | 5 +- src/lib/ares_parse_ptr_reply.c | 6 +-- src/lib/ares_parse_uri_reply.c | 3 +- src/lib/ares_platform.c | 2 +- src/lib/ares_private.h | 2 +- src/lib/ares_search.c | 6 +-- src/lib/{ares_strdup.c => ares_str.c} | 66 ++++++++++++++++++++------- src/lib/{ares_strdup.h => ares_str.h} | 17 ++++++- src/lib/ares_strsplit.c | 4 +- src/lib/inet_net_pton.c | 5 +- src/lib/inet_ntop.c | 6 +-- 21 files changed, 113 insertions(+), 64 deletions(-) rename src/lib/{ares_strdup.c => ares_str.c} (66%) rename src/lib/{ares_strdup.h => ares_str.h} (70%) diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index ef0a8aca..3a0ab80d 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -56,7 +56,7 @@ CSOURCES = ares__addrinfo2hostent.c \ ares_search.c \ ares_send.c \ ares_strcasecmp.c \ - ares_strdup.c \ + ares_str.c \ ares_strerror.c \ ares_strsplit.c \ ares_timeout.c \ @@ -82,7 +82,7 @@ HHEADERS = ares__htable.h \ ares_platform.h \ ares_private.h \ ares_strcasecmp.h \ - ares_strdup.h \ + ares_str.h \ ares_strsplit.h \ bitncmp.h \ ares_setup.h \ diff --git a/src/lib/ares__read_line.c b/src/lib/ares__read_line.c index f511dc41..6894916c 100644 --- a/src/lib/ares__read_line.c +++ b/src/lib/ares__read_line.c @@ -59,7 +59,7 @@ ares_status_t ares__read_line(FILE *fp, char **buf, size_t *bufsize) if (!fgets(*buf + offset, bytestoread, fp)) return (offset != 0) ? 0 : (ferror(fp)) ? ARES_EFILE : ARES_EOF; - len = offset + strlen(*buf + offset); + len = offset + ares_strlen(*buf + offset); if ((*buf)[len - 1] == '\n') { (*buf)[len - 1] = 0; diff --git a/src/lib/ares_android.c b/src/lib/ares_android.c index 13b89631..95ff9a02 100644 --- a/src/lib/ares_android.c +++ b/src/lib/ares_android.c @@ -331,9 +331,10 @@ char **ares_get_android_server_list(size_t max_servers, dns_list = ares_malloc(sizeof(*dns_list)*(*num_servers)); for (i=0; i<*num_servers; i++) { + size_t len = 64; server = (*env)->CallObjectMethod(env, server_list, android_list_get_mid, (jint)i); - dns_list[i] = ares_malloc(64); + dns_list[i] = ares_malloc(len); dns_list[i][0] = 0; if (server == NULL) { @@ -341,7 +342,7 @@ char **ares_get_android_server_list(size_t max_servers, } str = (*env)->CallObjectMethod(env, server, android_ia_host_addr_mid); ch_server_address = (*env)->GetStringUTFChars(env, str, 0); - strncpy(dns_list[i], ch_server_address, 64); + ares_strcpy(dns_list[i], ch_server_address, len); (*env)->ReleaseStringUTFChars(env, str, ch_server_address); (*env)->DeleteLocalRef(env, str); (*env)->DeleteLocalRef(env, server); diff --git a/src/lib/ares_create_query.c b/src/lib/ares_create_query.c index d5c1650b..00966d5e 100644 --- a/src/lib/ares_create_query.c +++ b/src/lib/ares_create_query.c @@ -108,7 +108,7 @@ int ares_create_query(const char *name, int dnsclass, int type, * is for the length byte and zero termination if no dots or ecscaping is * used. */ - len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ + + len = ares_strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ + (max_udp_size ? EDNSFIXEDSZ : 0); buf = ares_malloc(len); if (!buf) diff --git a/src/lib/ares_expand_string.c b/src/lib/ares_expand_string.c index ac734323..f54cd833 100644 --- a/src/lib/ares_expand_string.c +++ b/src/lib/ares_expand_string.c @@ -64,7 +64,7 @@ ares_status_t ares_expand_string_ex(const unsigned char *encoded, if (*s == NULL) return ARES_ENOMEM; q = *s; - strncpy((char *)q, (char *)encoded, len); + ares_strcpy((char *)q, (char *)encoded, len+1); q[len] = '\0'; *s = q; diff --git a/src/lib/ares_getaddrinfo.c b/src/lib/ares_getaddrinfo.c index 8de7b2a3..006c2294 100644 --- a/src/lib/ares_getaddrinfo.c +++ b/src/lib/ares_getaddrinfo.c @@ -430,7 +430,7 @@ static ares_bool_t is_localhost(const char *name) if (strcmp(name, "localhost") == 0) return ARES_TRUE; - len = strlen(name); + len = ares_strlen(name); if (len < 10 /* strlen(".localhost") */) return ARES_FALSE; @@ -855,7 +855,7 @@ static ares_bool_t as_is_first(const struct host_query* hquery) { char* p; size_t ndots = 0; - size_t nname = hquery->name?strlen(hquery->name):0; + size_t nname = ares_strlen(hquery->name); for (p = hquery->name; p && *p; p++) { if (*p == '.') @@ -873,7 +873,7 @@ static ares_bool_t as_is_first(const struct host_query* hquery) static ares_bool_t as_is_only(const struct host_query* hquery) { - size_t nname = hquery->name?strlen(hquery->name):0; + size_t nname = ares_strlen(hquery->name); if (nname && hquery->name[nname-1] == '.') return ARES_TRUE; return ARES_FALSE; diff --git a/src/lib/ares_gethostbyaddr.c b/src/lib/ares_gethostbyaddr.c index 6da8a873..13181a53 100644 --- a/src/lib/ares_gethostbyaddr.c +++ b/src/lib/ares_gethostbyaddr.c @@ -289,7 +289,7 @@ static void ptr_rr_name(char *name, size_t name_size, const struct ares_addr *ad bytes[13]&0xf, bytes[13] >> 4, bytes[12]&0xf, bytes[12] >> 4, bytes[11]&0xf, bytes[11] >> 4, bytes[10]&0xf, bytes[10] >> 4, bytes[9]&0xf, bytes[9] >> 4, bytes[8]&0xf, bytes[8] >> 4); - snprintf(name+strlen(name), name_size-strlen(name), + snprintf(name+ares_strlen(name), name_size-ares_strlen(name), "%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.ip6.arpa", bytes[7]&0xf, bytes[7] >> 4, bytes[6]&0xf, bytes[6] >> 4, bytes[5]&0xf, bytes[5] >> 4, bytes[4]&0xf, bytes[4] >> 4, diff --git a/src/lib/ares_getnameinfo.c b/src/lib/ares_getnameinfo.c index 69779f70..263c749f 100644 --- a/src/lib/ares_getnameinfo.c +++ b/src/lib/ares_getnameinfo.c @@ -341,7 +341,7 @@ static char *lookup_service(unsigned short port, unsigned int flags, snprintf(tmpbuf, sizeof(tmpbuf), "%u", (unsigned int)ntohs(port)); name = tmpbuf; } - name_len = strlen(name); + name_len = ares_strlen(name); if (name_len < buflen) /* return it if buffer big enough */ memcpy(buf, name, name_len + 1); @@ -408,11 +408,11 @@ static void append_scopeid(struct sockaddr_in6 *addr6, unsigned int flags, (void) flags; #endif tmpbuf[IF_NAMESIZE + 1] = '\0'; - bufl = strlen(buf); + bufl = ares_strlen(buf); - if(bufl + strlen(tmpbuf) < buflen) + if(bufl + ares_strlen(tmpbuf) < buflen) /* only append the scopeid string if it fits in the target buffer */ - strcpy(&buf[bufl], tmpbuf); + ares_strcpy(&buf[bufl], tmpbuf, buflen - bufl); } #endif @@ -421,7 +421,7 @@ STATIC_TESTABLE char *ares_striendstr(const char *s1, const char *s2) { const char *c1, *c2, *c1_begin; int lo1, lo2; - size_t s1_len = strlen(s1), s2_len = strlen(s2); + size_t s1_len = ares_strlen(s1), s2_len = ares_strlen(s2); /* If the substr is longer than the full str, it can't match */ if (s2_len > s1_len) diff --git a/src/lib/ares_init.c b/src/lib/ares_init.c index 0fb8393e..f7f58d26 100644 --- a/src/lib/ares_init.c +++ b/src/lib/ares_init.c @@ -296,8 +296,8 @@ int ares_dup(ares_channel *dest, ares_channel src) (*dest)->sock_funcs = src->sock_funcs; (*dest)->sock_func_cb_data = src->sock_func_cb_data; - strncpy((*dest)->local_dev_name, src->local_dev_name, - sizeof((*dest)->local_dev_name)); + ares_strcpy((*dest)->local_dev_name, src->local_dev_name, + sizeof((*dest)->local_dev_name)); (*dest)->local_ip4 = src->local_ip4; memcpy((*dest)->local_ip6, src->local_ip6, sizeof(src->local_ip6)); @@ -668,14 +668,14 @@ static void commanjoin(char** dst, const char* const src, const size_t len) size_t newsize; /* 1 for terminating 0 and 2 for , and terminating 0 */ - newsize = len + (*dst ? (strlen(*dst) + 2) : 1); + newsize = len + (*dst ? (ares_strlen(*dst) + 2) : 1); newbuf = ares_realloc(*dst, newsize); if (!newbuf) return; if (*dst == NULL) *newbuf = '\0'; *dst = newbuf; - if (strlen(*dst) != 0) + if (ares_strlen(*dst) != 0) strcat(*dst, ","); strncat(*dst, src, len); } @@ -687,7 +687,7 @@ static void commanjoin(char** dst, const char* const src, const size_t len) */ static void commajoin(char **dst, const char *src) { - commanjoin(dst, src, strlen(src)); + commanjoin(dst, src, ares_strlen(src)); } @@ -2215,7 +2215,7 @@ static ares_status_t set_options(ares_channel channel, const char *str) static const char *try_option(const char *p, const char *q, const char *opt) { - size_t len = strlen(opt); + size_t len = ares_strlen(opt); return ((size_t)(q - p) >= len && !strncmp(p, opt, len)) ? &p[len] : NULL; } @@ -2259,7 +2259,7 @@ static char *try_config(char *s, const char *opt, char scc) /* empty line */ return NULL; - if ((len = strlen(opt)) == 0) + if ((len = ares_strlen(opt)) == 0) /* empty option */ return NULL; /* LCOV_EXCL_LINE */ @@ -2355,8 +2355,8 @@ void ares_set_local_ip6(ares_channel channel, void ares_set_local_dev(ares_channel channel, const char* local_dev_name) { - strncpy(channel->local_dev_name, local_dev_name, - sizeof(channel->local_dev_name)); + ares_strcpy(channel->local_dev_name, local_dev_name, + sizeof(channel->local_dev_name)); channel->local_dev_name[sizeof(channel->local_dev_name) - 1] = 0; } diff --git a/src/lib/ares_options.c b/src/lib/ares_options.c index 756f1cd4..065f6721 100644 --- a/src/lib/ares_options.c +++ b/src/lib/ares_options.c @@ -275,7 +275,7 @@ static ares_status_t set_servers_csv(ares_channel channel, if (!channel) return ARES_ENODATA; - i = strlen(_csv); + i = ares_strlen(_csv); if (i == 0) return ARES_SUCCESS; /* blank all servers */ @@ -283,7 +283,7 @@ static ares_status_t set_servers_csv(ares_channel channel, if (!csv) return ARES_ENOMEM; - strcpy(csv, _csv); + ares_strcpy(csv, _csv, i + 2); if (csv[i-1] != ',') { /* make parsing easier by ensuring ending ',' */ csv[i] = ','; csv[i+1] = 0; diff --git a/src/lib/ares_parse_ns_reply.c b/src/lib/ares_parse_ns_reply.c index 5e7624e4..9e9ac720 100644 --- a/src/lib/ares_parse_ns_reply.c +++ b/src/lib/ares_parse_ns_reply.c @@ -138,7 +138,8 @@ int ares_parse_ns_reply( const unsigned char* abuf, int alen_int, break; } - nameservers[nameservers_num] = ares_malloc(strlen(rr_data)+1); + len = ares_strlen(rr_data)+1; + nameservers[nameservers_num] = ares_malloc(len); if (nameservers[nameservers_num]==NULL) { @@ -147,7 +148,7 @@ int ares_parse_ns_reply( const unsigned char* abuf, int alen_int, status=ARES_ENOMEM; break; } - strcpy(nameservers[nameservers_num],rr_data); + ares_strcpy(nameservers[nameservers_num], rr_data, len); ares_free(rr_data); nameservers_num++; diff --git a/src/lib/ares_parse_ptr_reply.c b/src/lib/ares_parse_ptr_reply.c index 648b3173..fe09b1ff 100644 --- a/src/lib/ares_parse_ptr_reply.c +++ b/src/lib/ares_parse_ptr_reply.c @@ -139,15 +139,15 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen_int, const void *ad if (hostname) ares_free(hostname); hostname = rr_data; - rr_data_len = strlen(rr_data)+1; - aliases[aliascnt] = ares_malloc(rr_data_len * sizeof(char)); + rr_data_len = ares_strlen(rr_data)+1; + aliases[aliascnt] = ares_malloc(rr_data_len); if (!aliases[aliascnt]) { ares_free(rr_name); status = ARES_ENOMEM; break; } - strncpy(aliases[aliascnt], rr_data, rr_data_len); + ares_strcpy(aliases[aliascnt], rr_data, rr_data_len); aliascnt++; if (aliascnt >= alias_alloc) { char **ptr; diff --git a/src/lib/ares_parse_uri_reply.c b/src/lib/ares_parse_uri_reply.c index 79d2e5b9..bb9486d8 100644 --- a/src/lib/ares_parse_uri_reply.c +++ b/src/lib/ares_parse_uri_reply.c @@ -166,8 +166,7 @@ ares_parse_uri_reply (const unsigned char *abuf, int alen_int, status = ARES_ENOMEM; break; } - uri_curr->uri = strncpy(uri_curr->uri, (const char *)vptr, rr_len-4); - uri_curr->uri[rr_len-4]='\0'; + ares_strcpy(uri_curr->uri, (const char *)vptr, rr_len-3); uri_curr->ttl = (int)rr_ttl; } diff --git a/src/lib/ares_platform.c b/src/lib/ares_platform.c index c13b9e52..8bdffb8b 100644 --- a/src/lib/ares_platform.c +++ b/src/lib/ares_platform.c @@ -11010,7 +11010,7 @@ struct servent *getservbyport(int port, const char *proto) if (proto) { - switch (strlen(proto)) { + switch (ares_strlen(proto)) { case 3: if (!strncasecmp(proto, "tcp", 3)) protocol = "tcp"; diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h index 670ed37f..3f5e567a 100644 --- a/src/lib/ares_private.h +++ b/src/lib/ares_private.h @@ -124,7 +124,7 @@ typedef struct ares_rand_state ares_rand_state; # define getenv(ptr) ares_getenv(ptr) #endif -#include "ares_strdup.h" +#include "ares_str.h" #include "ares_strsplit.h" #ifndef HAVE_STRCASECMP diff --git a/src/lib/ares_search.c b/src/lib/ares_search.c index 3f2460f9..efd51cb3 100644 --- a/src/lib/ares_search.c +++ b/src/lib/ares_search.c @@ -226,8 +226,8 @@ static void end_squery(struct search_query *squery, ares_status_t status, /* Concatenate two domains. */ ares_status_t ares__cat_domain(const char *name, const char *domain, char **s) { - size_t nlen = strlen(name); - size_t dlen = strlen(domain); + size_t nlen = ares_strlen(name); + size_t dlen = ares_strlen(domain); *s = ares_malloc(nlen + 1 + dlen + 1); if (!*s) @@ -251,7 +251,7 @@ ares_status_t ares__cat_domain(const char *name, const char *domain, char **s) ares_status_t ares__single_domain(ares_channel channel, const char *name, char **s) { - size_t len = strlen(name); + size_t len = ares_strlen(name); const char *hostaliases; FILE *fp; char *line = NULL; diff --git a/src/lib/ares_strdup.c b/src/lib/ares_str.c similarity index 66% rename from src/lib/ares_strdup.c rename to src/lib/ares_str.c index db5dd1d8..6883181d 100644 --- a/src/lib/ares_strdup.c +++ b/src/lib/ares_str.c @@ -26,27 +26,59 @@ */ #include "ares_setup.h" -#include "ares_strdup.h" +#include "ares_str.h" #include "ares.h" #include "ares_private.h" +size_t ares_strlen(const char *str) +{ + if (str == NULL) + return 0; + + return strlen(str); +} + char *ares_strdup(const char *s1) { - size_t sz; - char * s2; - - if(s1) { - sz = strlen(s1); - if(sz < (size_t)-1) { - sz++; - if(sz < ((size_t)-1)) { - s2 = ares_malloc(sz); - if(s2) { - memcpy(s2, s1, sz); - return s2; - } - } - } + size_t len; + char *out; + + if (s1 == NULL) + return NULL; + + len = ares_strlen(s1); + + /* Don't see how this is possible */ + if (len == SIZE_MAX) + return NULL; + + out = ares_malloc(len+1); + if (out == NULL) + return NULL; + + if (len) + memcpy(out, s1, len); + + out[len] = 0; + return out; +} + +size_t ares_strcpy(char *dest, const char *src, size_t dest_size) +{ + size_t len = 0; + + if (dest == NULL || dest_size == 0) + return 0; + + if (src != NULL) + len = strlen(src); + + if (len >= dest_size) + len = dest_size - 1; + + if (len) { + memcpy(dest, src, len); } - return (char *)NULL; + + return len; } diff --git a/src/lib/ares_strdup.h b/src/lib/ares_str.h similarity index 70% rename from src/lib/ares_strdup.h rename to src/lib/ares_str.h index 06e8cdcc..b1f99911 100644 --- a/src/lib/ares_strdup.h +++ b/src/lib/ares_str.h @@ -29,6 +29,21 @@ #include "ares_setup.h" -extern char *ares_strdup(const char *s1); +char *ares_strdup(const char *s1); + +size_t ares_strlen(const char *str); + +/*! Copy string from source to destination with destination buffer size + * provided. The destination is guaranteed to be null terminated, if the + * provided buffer isn't large enough, only those bytes from the source that + * will fit will be copied. + * + * \param[out] dest Destination buffer + * \param[in] src Source to copy + * \param[in] dest_size Size of destination buffer + * \return String length. Will be at most dest_size-1 + */ +size_t ares_strcpy(char *dest, const char *src, size_t dest_size); + #endif /* HEADER_CARES_STRDUP_H */ diff --git a/src/lib/ares_strsplit.c b/src/lib/ares_strsplit.c index 985a02db..eb496aad 100644 --- a/src/lib/ares_strsplit.c +++ b/src/lib/ares_strsplit.c @@ -89,8 +89,8 @@ char **ares__strsplit(const char *in, const char *delms, size_t *num_elm) { ares__strsplit_free(table, j); return NULL; } - strncpy(table[j], p, i); - table[j++][i] = 0; + ares_strcpy(table[j], p, i+1); + j++; } else count--; } diff --git a/src/lib/inet_net_pton.c b/src/lib/inet_net_pton.c index c4907c51..8f9e7829 100644 --- a/src/lib/inet_net_pton.c +++ b/src/lib/inet_net_pton.c @@ -33,6 +33,7 @@ #include "ares_ipv6.h" #include "ares_nowarn.h" #include "ares_inet_net_pton.h" +#include "ares_private.h" const struct ares_in6_addr ares_in6addr_any = { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }; @@ -322,11 +323,11 @@ ares_inet_net_pton_ipv6(const char *src, unsigned char *dst, size_t size) char buf[INET6_ADDRSTRLEN + sizeof("/128")]; char *sep; - if (strlen(src) >= sizeof buf) { + if (ares_strlen(src) >= sizeof buf) { SET_ERRNO(EMSGSIZE); return (-1); } - strncpy(buf, src, sizeof buf); + ares_strcpy(buf, src, sizeof buf); sep = strchr(buf, '/'); if (sep != NULL) diff --git a/src/lib/inet_ntop.c b/src/lib/inet_ntop.c index 246d6306..5137d714 100644 --- a/src/lib/inet_ntop.c +++ b/src/lib/inet_ntop.c @@ -90,7 +90,7 @@ inet_ntop4(const unsigned char *src, char *dst, size_t size) SET_ERRNO(ENOSPC); return (NULL); } - strcpy(dst, tmp); + ares_strcpy(dst, tmp, size); return (dst); } @@ -170,7 +170,7 @@ inet_ntop6(const unsigned char *src, char *dst, size_t size) (best.len == 5 && words[5] == 0xffff))) { if (!inet_ntop4(src+12, tp, sizeof(tmp) - (tp - tmp))) return (NULL); - tp += strlen(tp); + tp += ares_strlen(tp); break; } tp += snprintf(tp, sizeof(tmp)-(tp-tmp), "%x", words[i]); @@ -187,7 +187,7 @@ inet_ntop6(const unsigned char *src, char *dst, size_t size) SET_ERRNO(ENOSPC); return (NULL); } - strcpy(dst, tmp); + ares_strcpy(dst, tmp, size); return (dst); }