CA-366761: replace use of strncpy with inlined wrapper

Signed-off-by: Mark Syms <mark.syms@citrix.com>
jandryuk · May 13, 2022 · 6ffa1d8 · 6ffa1d8
1 parent 582ef71
commit 6ffa1d8
Show file tree

Hide file tree

Showing 18 changed files with 61 additions and 55 deletions.
diff --git a/control/tap-ctl-ipc.c b/control/tap-ctl-ipc.c
@@ -46,6 +46,7 @@
 #include "tap-ctl.h"
 #include "blktap2.h"
 #include "compiler.h"
+#include "util.h"
 
 int tap_ctl_debug = 0;
 
@@ -253,8 +254,7 @@ tap_ctl_connect(const char *name, int *sfd)
 		return -ENAMETOOLONG;
 	}
 
-	strncpy(saddr.sun_path, name, sizeof(saddr.sun_path));
-	saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0';
+	safe_strncpy(saddr.sun_path, name, sizeof(saddr.sun_path));
 
 	err = connect(fd, (const struct sockaddr *)&saddr, sizeof(saddr));
 	if (err) {

diff --git a/control/tap-ctl-unpause.c b/control/tap-ctl-unpause.c
@@ -40,6 +40,7 @@
 #include <getopt.h>
 
 #include "tap-ctl.h"
+#include "util.h"
 
 int
 tap_ctl_unpause(const int id, const int minor, const char *params, int flags,
@@ -54,8 +55,8 @@ tap_ctl_unpause(const int id, const int minor, const char *params, int flags,
 	message.u.params.flags = flags;
 
 	if (params)
-		strncpy(message.u.params.path, params,
-				sizeof(message.u.params.path) - 1);
+		safe_strncpy(message.u.params.path, params,
+			     sizeof(message.u.params.path));
 	if (secondary) {
 		err = snprintf(message.u.params.secondary,
 				sizeof(message.u.params.secondary) - 1, "%s",

diff --git a/control/tap-ctl-xen.c b/control/tap-ctl-xen.c
@@ -47,6 +47,7 @@
 
 #include "tap-ctl.h"
 #include "compiler.h"
+#include "util.h"
 
 int
 tap_ctl_connect_xenblkif(const pid_t pid, const domid_t domid, const int devid, int poll_duration,
@@ -75,8 +76,7 @@ tap_ctl_connect_xenblkif(const pid_t pid, const domid_t domid, const int devid,
             EPRINTF("pool name too long: %s\n", pool);
             return -ENAMETOOLONG;
         }
-        strncpy(message.u.blkif.pool, pool, sizeof(message.u.blkif.pool));
-        message.u.blkif.pool[sizeof(message.u.blkif.pool) - 1] = '\0';
+        safe_strncpy(message.u.blkif.pool, pool, sizeof(message.u.blkif.pool));
     } else {
         message.u.blkif.pool[0] = 0;
     }

diff --git a/drivers/block-crypto.c b/drivers/block-crypto.c
@@ -140,11 +140,11 @@ find_keyfile(char **keyfile, const char *dirs,
 		sep = strchr(dirs, ',');
 		/* get directory element */
 		if (sep == NULL) {
-			strncpy(keydir, dirs, sizeof(keydir));
+			safe_strncpy(keydir, dirs, sizeof(keydir));
 			dirs = NULL;
 		} else {
 			size_t len = sep - dirs;
-			strncpy(keydir, dirs, len);
+			safe_strncpy(keydir, dirs, len);
 			dirs = sep+1;
 		}
 

diff --git a/drivers/block-nbd.c b/drivers/block-nbd.c
@@ -51,6 +51,7 @@
 #include "timeout-math.h"
 #include "tapdisk-nbdserver.h"
 #include "tapdisk-protocol-new.h"
+#include "util.h"
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -199,9 +200,8 @@ tdnbd_stash_passed_fd(int fd, char *msg, void *data)
 		close(passed_fds[free_index].fd);
 
 	passed_fds[free_index].fd = fd;
-	strncpy(passed_fds[free_index].id, msg,
-		sizeof(passed_fds[free_index].id) - 1);
-	passed_fds[free_index].id[sizeof(passed_fds[free_index].id) - 1] = '\0';
+	safe_strncpy(passed_fds[free_index].id, msg,
+		     sizeof(passed_fds[free_index].id));
 }
 
 static int
@@ -921,8 +921,7 @@ tdnbd_open(td_driver_t* driver, const char* name,
 			return -1;
 		}
 		prv->remote_un.sun_family = AF_UNIX;
-		strncpy(prv->remote_un.sun_path, name, sizeof(prv->remote_un.sun_path));
-		prv->remote_un.sun_path[sizeof(prv->remote_un.sun_path) - 1] = '\0';
+		safe_strncpy(prv->remote_un.sun_path, name, sizeof(prv->remote_un.sun_path));
 		len = strlen(prv->remote_un.sun_path)
 			+ sizeof(prv->remote_un.sun_family);
 		if ((rc = connect(prv->socket, (struct sockaddr*)&prv->remote_un, len)

diff --git a/drivers/block-valve.c b/drivers/block-valve.c
@@ -232,8 +232,7 @@ valve_sock_open(td_valve_t *valve)
 			err = -ENAMETOOLONG;
 			goto fail;
 		}
-		strncpy(addr.sun_path, valve->brname, sizeof(addr.sun_path));
-		addr.sun_path[sizeof(addr.sun_path) - 1] = '\0';
+		safe_strncpy(addr.sun_path, valve->brname, sizeof(addr.sun_path));
 	} else
 		snprintf(addr.sun_path, sizeof(addr.sun_path),
 			 "%s/%s", TD_VALVE_SOCKDIR, valve->brname);

diff --git a/drivers/tapdisk-control.c b/drivers/tapdisk-control.c
@@ -61,6 +61,7 @@
 #include "tapdisk-nbdserver.h"
 #include "td-blkif.h"
 #include "timeout-math.h"
+#include "util.h"
 
 #define TD_CTL_MAX_CONNECTIONS  10
 #define TD_CTL_SOCK_BACKLOG     32
@@ -573,8 +574,8 @@ tapdisk_control_list(struct tapdisk_ctl_conn *conn,
 		response->u.list.path[0] = 0;
 
 		if (vbd->name)
-			strncpy(response->u.list.path, vbd->name,
-				sizeof(response->u.list.path));
+			safe_strncpy(response->u.list.path, vbd->name,
+				     sizeof(response->u.list.path));
 
 		tapdisk_control_write_message(conn, response);
 	}
@@ -1624,8 +1625,7 @@ tapdisk_control_create_socket(char **socket_path)
 		err = ENAMETOOLONG;
 		goto fail;
 	}
-	strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path));
-	saddr.sun_path[sizeof(saddr.sun_path) - 1] = '\0';
+	safe_strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path));
 
 	saddr.sun_family = AF_UNIX;
 

diff --git a/drivers/tapdisk-disktype.c b/drivers/tapdisk-disktype.c
@@ -248,7 +248,7 @@ tapdisk_disktype_parse_params(const char *params, const char **_path)
 		return -ENAMETOOLONG;
 
 	memset(name, 0, sizeof(name));
-	strncpy(name, params, len);
+	safe_strncpy(name, params, len);
 
 	type = tapdisk_disktype_find(name);
 

diff --git a/drivers/tapdisk-nbdserver.c b/drivers/tapdisk-nbdserver.c
@@ -54,6 +54,7 @@
 #include "tapdisk-fdreceiver.h"
 
 #include "timeout-math.h"
+#include "util.h"
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -1379,8 +1380,7 @@ tapdisk_nbdserver_listen_unix(td_nbdserver_t *server)
 		goto out;
 	}
 
-	strncpy(server->local.sun_path, server->sockpath, sizeof(server->local.sun_path));
-	server->local.sun_path[sizeof(server->local.sun_path) - 1] = '\0';
+	safe_strncpy(server->local.sun_path, server->sockpath, sizeof(server->local.sun_path));
 	err = unlink(server->local.sun_path);
 	if (err == -1 && errno != ENOENT) {
 		err = -errno;

diff --git a/drivers/td-rated.c b/drivers/td-rated.c
@@ -407,8 +407,7 @@ rlb_sock_open(td_rlb_t *rlb)
 			err = -ENAMETOOLONG;
 			goto fail;
 		}
-		strncpy(rlb->addr.sun_path, rlb->name, sizeof(rlb->addr.sun_path));
-		rlb->addr.sun_path[sizeof(rlb->addr.sun_path) - 1] = '\0';
+		safe_strncpy(rlb->addr.sun_path, rlb->name, sizeof(rlb->addr.sun_path));
 	} else
 		snprintf(rlb->addr.sun_path, sizeof(rlb->addr.sun_path),
 			 "%s/%s", TD_VALVE_SOCKDIR, rlb->name);

diff --git a/include/util.h b/include/util.h
@@ -31,6 +31,22 @@
 #ifndef __TAPDISK_UTIL_H__
 #define __TAPDISK_UTIL_H_
 
+#include <stddef.h>
+#include <string.h>
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
+/*
+ * Strncpy variant that guarantees to terminate the string
+ */
+static inline char *
+safe_strncpy(char *dest, const char *src, size_t n)
+{
+	char *pdest;
+	pdest = strncpy(dest, src, n);
+	if (n > 0)
+		dest[n - 1] = '\0';
+	return pdest;
+}
+
 #endif /* __TAPDISK_UTIL_H__ */
diff --git a/lvm/Makefile.am b/lvm/Makefile.am
@@ -13,6 +13,7 @@ lvm_util_SOURCES = main.c
 lvm_util_LDADD = liblvmutil.la
 
 liblvmutil_la_CPPFLAGS  = -D_GNU_SOURCE
+liblvmutil_la_CPPFLAGS += -I$(top_srcdir)/include
 
 clean-local:
 	-rm -rf *.gc??
diff --git a/lvm/lvm-util.c b/lvm/lvm-util.c
@@ -39,6 +39,7 @@
 #include <syslog.h>
 
 #include "lvm-util.h"
+#include "util.h"
 
 #define EPRINTF(_f, _a...)					\
 	do {							\
@@ -67,8 +68,7 @@ lvm_copy_name(char *dst, const char *src, size_t size)
 	if (strnlen(src, size) == size)
 		return -ENAMETOOLONG;
 
-	strncpy(dst, src, size);
-	dst[size - 1] = '\0';
+	safe_strncpy(dst, src, size);
 	return 0;
 }
 

diff --git a/vhd/lib/canonpath.c b/vhd/lib/canonpath.c
@@ -44,6 +44,7 @@
 #include <unistd.h>
 
 #include "canonpath.h"
+#include "util.h"
 
 char *
 normalize_path(const char *path, size_t path_len)
@@ -140,8 +141,7 @@ canonpath(const char *path, char *resolved_path, size_t dest_size)
 	if (strncmp(canon, dev_mapper_path, dev_mapper_len) == 0 &&
 	    strchr(canon + dev_mapper_len, '/') == NULL &&
 	    access(canon, F_OK) == 0) {
-		strncpy(resolved_path, canon, dest_size);
-		resolved_path[dest_size - 1] = '\0';
+		safe_strncpy(resolved_path, canon, dest_size);
 		goto end;
 	}
 
@@ -153,8 +153,7 @@ canonpath(const char *path, char *resolved_path, size_t dest_size)
 	 */
 	if (strncmp(canon, dev_path, dev_len) == 0 && (p = strchr(canon + dev_len, '/')) != NULL) {
 		if (strchr(p+1, '/') == NULL) {
-			strncpy(resolved_path, dev_mapper_path, dest_size);
-			resolved_path[dest_size - 1] = '\0';
+			safe_strncpy(resolved_path, dev_mapper_path, dest_size);
 			dst = strchr(resolved_path, 0);
 			for (p = canon + dev_len; *p; ++p) {
 				if (dst - resolved_path >= PATH_MAX - 2)
@@ -190,8 +189,7 @@ canonpath(const char *path, char *resolved_path, size_t dest_size)
 			if (p == resolved_path + dev_drbd_prefix_len || errno == ERANGE || *p != '\0')
 				goto end; /* Cannot parse correctly pattern. */
 
-			strncpy(resolved_path, canon, dest_size);
-			resolved_path[dest_size - 1] = '\0';
+			safe_strncpy(resolved_path, canon, dest_size);
 		} else
 			goto fallback;
 		if (access(resolved_path, F_OK) == 0)

diff --git a/vhd/lib/libvhd-index.c b/vhd/lib/libvhd-index.c
@@ -47,6 +47,7 @@
 #include "libvhd-index.h"
 #include "relative-path.h"
 #include "canonpath.h"
+#include "util.h"
 
 typedef struct vhdi_path                    vhdi_path_t;
 typedef struct vhdi_header                  vhdi_header_t;
@@ -248,8 +249,7 @@ vhdi_path_expand(const char *src, vhdi_path_t *dest, int *err)
 	char *path, *base, copy[VHD_MAX_NAME_LEN];
 	char *absolute_path, __absolute_path[PATH_MAX];
 
-	strncpy(copy, src, sizeof(copy));
-	copy[sizeof(copy) - 1] = '\0';
+	safe_strncpy(copy, src, sizeof(copy));
 	base = dirname(copy);
 
 	*err = asprintf(&path, "%s/%s", base, dest->path);
@@ -688,8 +688,7 @@ vhdi_copy_path_to(vhdi_path_t *path, const char *src, const char *dest, size_t d
 	char *file, *relative_path, copy[VHD_MAX_NAME_LEN];
 	char *absolute_path, __absolute_path[PATH_MAX];
 
-	strncpy(copy, dest, sizeof(copy));
-	copy[sizeof(copy) - 1] = '\0';
+	safe_strncpy(copy, dest, sizeof(copy));
 
 	file          = basename(copy);
 	absolute_path = canonpath(copy, __absolute_path, sizeof(__absolute_path));
@@ -717,8 +716,7 @@ vhdi_copy_path_to(vhdi_path_t *path, const char *src, const char *dest, size_t d
 		goto out;
 	}
 
-	strncpy(path->path, relative_path, dest_size);
-	path->path[dest_size - 1] = '\0';
+	safe_strncpy(path->path, relative_path, dest_size);
 	path->bytes = len + 1;
 
 	err = 0;
@@ -841,8 +839,7 @@ vhdi_bat_load(const char *name, vhdi_bat_t *bat)
 	path = vhdi_path_expand(name, &header.vhd_path, &err);
 	if (err)
 		goto out;
-	strncpy(bat->vhd_path, path, sizeof(bat->vhd_path));
-	bat->vhd_path[sizeof(bat->vhd_path) - 1] = '\0';
+	safe_strncpy(bat->vhd_path, path, sizeof(bat->vhd_path));
 	free(path);
 
 	err = access(bat->vhd_path, F_OK);
@@ -854,8 +851,7 @@ vhdi_bat_load(const char *name, vhdi_bat_t *bat)
 	path = vhdi_path_expand(name, &header.index_path, &err);
 	if (err)
 		goto out;
-	strncpy(bat->index_path, path, sizeof(bat->index_path));
-	bat->index_path[sizeof(bat->index_path) - 1] = '\0';
+	safe_strncpy(bat->index_path, path, sizeof(bat->index_path));
 	free(path);
 
 	err = access(bat->index_path, F_OK);
@@ -867,8 +863,7 @@ vhdi_bat_load(const char *name, vhdi_bat_t *bat)
 	path = vhdi_path_expand(name, &header.file_table_path, &err);
 	if (err)
 		goto out;
-	strncpy(bat->file_table_path, path, sizeof(bat->file_table_path));
-	bat->file_table_path[sizeof(bat->file_table_path) - 1] = '\0';
+	safe_strncpy(bat->file_table_path, path, sizeof(bat->file_table_path));
 	free(path);
 
 	err = access(bat->file_table_path, F_OK);

diff --git a/vhd/lib/libvhd.c b/vhd/lib/libvhd.c
@@ -57,6 +57,7 @@
 #include "relative-path.h"
 #include "canonpath.h"
 #include "compiler.h"
+#include "util.h"
 
 /* VHD uses an epoch of 12:00AM, Jan 1, 2000. This is the Unix timestamp for 
  * the start of the VHD epoch. */
@@ -2876,7 +2877,7 @@ vhd_initialize_footer(vhd_context_t *ctx, int type, uint64_t size)
 	ctx->footer.type         = type;
 	ctx->footer.saved        = 0;
 	ctx->footer.data_offset  = 0xFFFFFFFFFFFFFFFFULL;
-	strncpy(ctx->footer.crtr_app, "tap", sizeof(ctx->footer.crtr_app));
+	safe_strncpy(ctx->footer.crtr_app, "tap", sizeof(ctx->footer.crtr_app));
 	uuid_generate(ctx->footer.uuid);
 }
 

diff --git a/vhd/lib/vhd-util-scan.c b/vhd/lib/vhd-util-scan.c
@@ -50,6 +50,7 @@
 #include "libvhd.h"
 #include "lvm-util.h"
 #include "canonpath.h"
+#include "util.h"
 
 #define VHD_SCAN_FAST        0x01
 #define VHD_SCAN_PRETTY      0x02
@@ -504,13 +505,11 @@ vhd_util_scan_extract_volume_name(char *dst, const char *src, size_t size)
 	c = strrchr(copy, '/');
 	if (c == name) {
 		/* unrecognized format */
-		strncpy(dst, src, size);
-		dst[size - 1] = '\0';
+		safe_strncpy(dst, src, size);
 		return -EINVAL;
 	}
 
-	strncpy(dst, ++c, size);
-	dst[size - 1] = '\0';
+	safe_strncpy(dst, ++c, size);
 	return 0;
 }
 

diff --git a/vhd/vhd-index.c b/vhd/vhd-index.c
@@ -42,6 +42,7 @@
 
 #include "libvhd.h"
 #include "libvhd-index.h"
+#include "util.h"
 
 static void
 usage(void)
@@ -413,12 +414,9 @@ vhd_index_add_bat(vhdi_name_t *name,
 	bat.vhd_blocks     = vhd_blocks;
 	bat.vhd_block_size = vhd_block_size;
 
-	strncpy(bat.vhd_path, name->vhd, sizeof(bat.vhd_path));
-	bat.vhd_path[sizeof(bat.vhd_path) - 1] = '\0';
-	strncpy(bat.index_path, name->index, sizeof(bat.index_path));
-	bat.index_path[sizeof(bat.index_path) - 1] = '\0';
-	strncpy(bat.file_table_path, name->files, sizeof(bat.file_table_path));
-	bat.file_table_path[sizeof(bat.file_table_path) - 1] = '\0';
+	safe_strncpy(bat.vhd_path, name->vhd, sizeof(bat.vhd_path));
+	safe_strncpy(bat.index_path, name->index, sizeof(bat.index_path));
+	safe_strncpy(bat.file_table_path, name->files, sizeof(bat.file_table_path));
 
 	err = vhdi_open(&vhdi, name->index, O_RDWR);
 	if (err)