Use strlcpy() instead of strncpy() (CID 246564)

..making sure dest buffer is always nul-terminated.

Author: edenhill

configure.self

@@ -204,6 +204,16 @@ int foo (void) {
    return strndup(\"hi\", 2) ? 0 : 1;
 }"
 
+    # Check if strlcpy() is available
+    mkl_compile_check "strlcpy" "HAVE_STRLCPY" disable CC "" \
+"
+#define _DARWIN_C_SOURCE
+#include <string.h>
+int foo (void) {
+    char dest[4];
+   return strlcpy(dest, \"something\", sizeof(dest));
+}"
+
     # Check if strerror_r() is available.
     # The check for GNU vs XSI is done in rdposix.h since
     # we can't rely on all defines to be set here (_GNU_SOURCE).

src/rd.h

@@ -34,6 +34,11 @@
 #ifndef _GNU_SOURCE
 #define _GNU_SOURCE  /* for strndup() */
 #endif
+
+#if defined(__APPLE__) && !defined(_DARWIN_C_SOURCE)
+#define _DARWIN_C_SOURCE /* for strlcpy, pthread_setname_np, etc */
+#endif
+
 #define __need_IOV_MAX
 #ifndef _POSIX_C_SOURCE
 #define _POSIX_C_SOURCE 200809L  /* for timespec on solaris */

src/rdkafka.c

@@ -238,8 +238,7 @@ static void rd_kafka_log_buf (const rd_kafka_conf_t *conf,
                 rko = rd_kafka_op_new(RD_KAFKA_OP_LOG);
                 rd_kafka_op_set_prio(rko, RD_KAFKA_PRIO_MEDIUM);
                 rko->rko_u.log.level = level;
-                strncpy(rko->rko_u.log.fac, fac,
-                        sizeof(rko->rko_u.log.fac) - 1);
+                rd_strlcpy(rko->rko_u.log.fac, fac, sizeof(rko->rko_u.log.fac));
                 rko->rko_u.log.str = rd_strdup(buf);
                 rd_kafka_q_enq(rk->rk_logq, rko);
 

src/rdkafka_broker.c

@@ -1812,7 +1812,7 @@ static int rd_kafka_broker_connect (rd_kafka_broker_t *rkb) {
         rd_atomic32_add(&rkb->rkb_c.connects, 1);
 
         rd_kafka_broker_lock(rkb);
-        strncpy(nodename, rkb->rkb_nodename, sizeof(nodename));
+        rd_strlcpy(nodename, rkb->rkb_nodename, sizeof(nodename));
         rkb->rkb_connect_epoch = rkb->rkb_nodename_epoch;
         /* Logical brokers might not have a hostname set, in which case
          * we should not try to connect. */
@@ -2618,9 +2618,9 @@ static int rd_kafka_broker_op_serve (rd_kafka_broker_t *rkb,
                                    "Nodename changed from %s to %s",
                                    rkb->rkb_nodename,
                                    rko->rko_u.node.nodename);
-                        strncpy(rkb->rkb_nodename,
-                                rko->rko_u.node.nodename,
-                                sizeof(rkb->rkb_nodename)-1);
+                        rd_strlcpy(rkb->rkb_nodename,
+                                   rko->rko_u.node.nodename,
+                                   sizeof(rkb->rkb_nodename));
                         rkb->rkb_nodename_epoch++;
                         updated |= _UPD_NAME;
                 }
@@ -2659,8 +2659,8 @@ static int rd_kafka_broker_op_serve (rd_kafka_broker_t *rkb,
                         rd_rkb_dbg(rkb, BROKER, "UPDATE",
                                    "Name changed from %s to %s",
                                    rkb->rkb_name, brokername);
-                        strncpy(rkb->rkb_name, brokername,
-                                sizeof(rkb->rkb_name)-1);
+                        rd_strlcpy(rkb->rkb_name, brokername,
+                                   sizeof(rkb->rkb_name));
                 }
                 rd_kafka_broker_unlock(rkb);
                 rd_kafka_wrunlock(rkb->rkb_rk);
@@ -4954,7 +4954,7 @@ void rd_kafka_broker_set_nodename (rd_kafka_broker_t *rkb,
         /* Get nodename from from_rkb */
         if (from_rkb) {
                 rd_kafka_broker_lock(from_rkb);
-                strncpy(nodename, from_rkb->rkb_nodename, sizeof(nodename));
+                rd_strlcpy(nodename, from_rkb->rkb_nodename, sizeof(nodename));
                 nodeid = from_rkb->rkb_nodeid;
                 rd_kafka_broker_unlock(from_rkb);
         } else {
@@ -4968,8 +4968,8 @@ void rd_kafka_broker_set_nodename (rd_kafka_broker_t *rkb,
                 rd_rkb_dbg(rkb, BROKER, "NODENAME",
                            "Broker nodename changed from \"%s\" to \"%s\"",
                            rkb->rkb_nodename, nodename);
-                strncpy(rkb->rkb_nodename, nodename,
-                        sizeof(rkb->rkb_nodename));
+                rd_strlcpy(rkb->rkb_nodename, nodename,
+                           sizeof(rkb->rkb_nodename));
                 rkb->rkb_nodename_epoch++;
                 changed = rd_true;
         }
@@ -5296,8 +5296,8 @@ void rd_kafka_broker_update (rd_kafka_t *rk, rd_kafka_secproto_t proto,
                         rd_kafka_op_t *rko;
 
                         rko = rd_kafka_op_new(RD_KAFKA_OP_NODE_UPDATE);
-                        strncpy(rko->rko_u.node.nodename, nodename,
-				sizeof(rko->rko_u.node.nodename)-1);
+                        rd_strlcpy(rko->rko_u.node.nodename, nodename,
+                                   sizeof(rko->rko_u.node.nodename));
                         rko->rko_u.node.nodeid   = mdb->id;
                         rd_kafka_q_enq(rkb->rkb_ops, rko);
                 }

src/rdkafka_int.h

@@ -595,8 +595,7 @@ void rd_kafka_log0(const rd_kafka_conf_t *conf,
 #define rd_rkb_log(rkb,level,fac,...) do {				\
 		char _logname[RD_KAFKA_NODENAME_SIZE];			\
                 mtx_lock(&(rkb)->rkb_logname_lock);                     \
-		strncpy(_logname, rkb->rkb_logname, sizeof(_logname)-1); \
-		_logname[RD_KAFKA_NODENAME_SIZE-1] = '\0';		\
+                rd_strlcpy(_logname, rkb->rkb_logname, sizeof(_logname)); \
                 mtx_unlock(&(rkb)->rkb_logname_lock);                   \
 		rd_kafka_log0(&(rkb)->rkb_rk->rk_conf, \
                               (rkb)->rkb_rk, _logname,                  \

src/rdregex.c

@@ -28,6 +28,7 @@
 
 
 #include "rd.h"
+#include "rdstring.h"
 #include "rdregex.h"
 
 #if HAVE_REGEX
@@ -80,10 +81,8 @@ rd_regex_comp (const char *pattern, char *errstr, size_t errstr_size) {
 
 	re->re = re_regcomp(pattern, 0, &errstr2);
 	if (!re->re) {
-		if (errstr) {
-			strncpy(errstr, errstr2, errstr_size-1);
-			errstr[errstr_size-1] = '\0';
-		}
+                if (errstr)
+                        rd_strlcpy(errstr, errstr2, errstr_size);
 		rd_free(re);
 		return NULL;
 	}
@@ -141,10 +140,8 @@ int rd_regex_match (const char *pattern, const char *str,
 	/* FIXME: cache compiled regex */
 	re = re_regcomp(pattern, 0, &errstr2);
 	if (!re) {
-		if (errstr) {
-			strncpy(errstr, errstr2, errstr_size-1);
-			errstr[errstr_size-1] = '\0';
-		}
+                if (errstr)
+                        rd_strlcpy(errstr, errstr2, errstr_size);
 		return -1;
 	}
 

src/rdstring.h

@@ -30,6 +30,17 @@
 #ifndef _RDSTRING_H_
 #define _RDSTRING_H_
 
+static RD_INLINE RD_UNUSED
+void rd_strlcpy (char *dst, const char *src, size_t dstsize) {
+#if HAVE_STRLCPY
+        (void)strlcpy(dst, src, dstsize);
+#else
+        if (likely(dstsize > 0))
+                strncpy(dst, src, dstsize-1);
+        dst[dstsize] = '\0';
+#endif
+}
+
 
 
 char *rd_string_render (const char *templ,

src/tinycthread_extra.c

@@ -31,10 +31,6 @@
  * @brief Extra methods added to tinycthread/c11threads
  */
 
-#if defined(__APPLE__) && !defined(_DARWIN_C_SOURCE)
-#define _DARWIN_C_SOURCE  /* for pthread_setname_np() on macOS */
-#endif
-
 #include "rd.h"
 #include "rdtime.h"
 #include "tinycthread.h"