[lxc-devel] [lxc/master] secure coding: #3 strcpy => strlcpy

[2xsec on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 414 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180618/7b2d4280/attachment.bin>
-------------- next part --------------
From 18cd4b5484e17573498bbf848f8671c5a0856a70 Mon Sep 17 00:00:00 2001
From: Donghwa Jeong <dh48.jeong at samsung.com>
Date: Mon, 18 Jun 2018 18:21:23 +0900
Subject: [PATCH] secure coding: #3 strcpy => strlcpy

Signed-off-by: Donghwa Jeong <dh48.jeong at samsung.com>
---
 src/lxc/confile.c       |  6 +++---
 src/lxc/confile_utils.c | 17 +++++++++++++----
 src/lxc/confile_utils.h |  2 +-
 src/tests/attach.c      |  7 ++++++-
 src/tests/cgpath.c      |  6 +++++-
 src/tests/shortlived.c  |  7 ++++++-
 6 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index d019c8984..82ee093fd 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -450,7 +450,7 @@ static int set_config_net_link(const char *key, const char *value,
 	if (value[strlen(value) - 1] == '+' && netdev->type == LXC_NET_PHYS)
 		ret = create_matched_ifnames(value, lxc_conf, netdev);
 	else
-		ret = network_ifname(netdev->link, value);
+		ret = network_ifname(netdev->link, value, sizeof(netdev->link));
 
 	return ret;
 }
@@ -466,7 +466,7 @@ static int set_config_net_name(const char *key, const char *value,
 	if (!netdev)
 		return -1;
 
-	return network_ifname(netdev->name, value);
+	return network_ifname(netdev->name, value, sizeof(netdev->name));
 }
 
 static int set_config_net_veth_pair(const char *key, const char *value,
@@ -480,7 +480,7 @@ static int set_config_net_veth_pair(const char *key, const char *value,
 	if (!netdev)
 		return -1;
 
-	return network_ifname(netdev->priv.veth_attr.pair, value);
+	return network_ifname(netdev->priv.veth_attr.pair, value, sizeof(netdev->priv.veth_attr.pair));
 }
 
 static int set_config_net_macvlan_mode(const char *key, const char *value,
diff --git a/src/lxc/confile_utils.c b/src/lxc/confile_utils.c
index 5686c60e9..30df78d94 100644
--- a/src/lxc/confile_utils.c
+++ b/src/lxc/confile_utils.c
@@ -36,6 +36,10 @@
 #include "parse.h"
 #include "utils.h"
 
+#ifndef HAVE_STRLCPY
+#include "include/strlcpy.h"
+#endif
+
 lxc_log_define(lxc_confile_utils, lxc);
 
 int parse_idmaps(const char *idmap, char *type, unsigned long *nsid,
@@ -509,14 +513,19 @@ int config_ip_prefix(struct in_addr *addr)
 	return 0;
 }
 
-int network_ifname(char *valuep, const char *value)
+int network_ifname(char *valuep, const char *value, size_t size)
 {
-	if (strlen(value) >= IFNAMSIZ) {
+	size_t retlen;
+
+	if (!valuep || !value)
+		return -1;
+
+	retlen = strlcpy(valuep, value, size);
+	if (retlen >= size) {
 		ERROR("Network devie name \"%s\" is too long (>= %zu)", value,
-		      (size_t)IFNAMSIZ);
+		      size);
 	}
 
-	strcpy(valuep, value);
 	return 0;
 }
 
diff --git a/src/lxc/confile_utils.h b/src/lxc/confile_utils.h
index 1e20c4f5f..a5b76820e 100644
--- a/src/lxc/confile_utils.h
+++ b/src/lxc/confile_utils.h
@@ -80,7 +80,7 @@ extern int set_config_string_item_max(char **conf_item, const char *value,
 				      size_t max);
 extern int set_config_path_item(char **conf_item, const char *value);
 extern int config_ip_prefix(struct in_addr *addr);
-extern int network_ifname(char *valuep, const char *value);
+extern int network_ifname(char *valuep, const char *value, size_t size);
 extern int rand_complete_hwaddr(char *hwaddr);
 extern bool lxc_config_net_hwaddr(const char *line);
 extern void update_hwaddr(const char *line);
diff --git a/src/tests/attach.c b/src/tests/attach.c
index 452ba8652..7a1519b9f 100644
--- a/src/tests/attach.c
+++ b/src/tests/attach.c
@@ -32,6 +32,10 @@
 
 #include <lxc/lxccontainer.h>
 
+#ifndef HAVE_STRLCPY
+#include "include/strlcpy.h"
+#endif
+
 #define TSTNAME    "lxc-attach-test"
 #define TSTOUT(fmt, ...) do { \
 	fprintf(stdout, fmt, ##__VA_ARGS__); fflush(NULL); \
@@ -399,7 +403,8 @@ int main(int argc, char *argv[])
 	char template[sizeof(P_tmpdir"/attach_XXXXXX")];
 	int fret = EXIT_FAILURE;
 
-	strcpy(template, P_tmpdir"/attach_XXXXXX");
+	(void)strlcpy(template, P_tmpdir"/attach_XXXXXX", sizeof(template));
+
 	i = lxc_make_tmpfile(template, false);
 	if (i < 0) {
 		lxc_error("Failed to create temporary log file for container %s\n", TSTNAME);
diff --git a/src/tests/cgpath.c b/src/tests/cgpath.c
index e794e565f..fa8d47678 100644
--- a/src/tests/cgpath.c
+++ b/src/tests/cgpath.c
@@ -33,6 +33,10 @@
 #include "lxc.h"
 #include "commands.h"
 
+#ifndef HAVE_STRLCPY
+#include "include/strlcpy.h"
+#endif
+
 #define MYNAME "lxctest1"
 
 #define TSTERR(fmt, ...) do { \
@@ -87,7 +91,7 @@ static int test_running_container(const char *lxcpath,
 		TSTERR("cgroup_get failed");
 		goto err3;
 	}
-	strcpy(value_save, value);
+	(void)strlcpy(value_save, value, NAME_MAX);
 
 	ret = cgroup_ops->set(cgroup_ops, "memory.soft_limit_in_bytes", "512M",
 			      c->name, c->config_path);
diff --git a/src/tests/shortlived.c b/src/tests/shortlived.c
index af5bb2eb7..2a039a2f3 100644
--- a/src/tests/shortlived.c
+++ b/src/tests/shortlived.c
@@ -31,6 +31,10 @@
 #include "lxctest.h"
 #include "utils.h"
 
+#ifndef HAVE_STRLCPY
+#include "include/strlcpy.h"
+#endif
+
 #define MYNAME "shortlived"
 
 static int destroy_container(void)
@@ -103,7 +107,8 @@ int main(int argc, char *argv[])
 	char template[sizeof(P_tmpdir"/shortlived_XXXXXX")];
 	int ret = EXIT_FAILURE;
 
-	strcpy(template, P_tmpdir"/shortlived_XXXXXX");
+	(void)strlcpy(template, P_tmpdir"/shortlived_XXXXXX", sizeof(template));
+
 	i = lxc_make_tmpfile(template, false);
 	if (i < 0) {
 		lxc_error("Failed to create temporary log file for container %s\n", MYNAME);