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

@@ -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				\

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;

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);

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)

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;

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;

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,

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)

src/lib/ares_init.c

@@ -296,7 +296,7 @@ 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,
+  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,7 +2355,7 @@ 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,
+  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;
 }

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;

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++;

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;

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;
         }
 

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";

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

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;

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;
 }

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 */

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--;
     }

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)

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);
 }