public inbox for b.a.t.m.a.n@lists.open-mesh.org
 help / color / mirror / Atom feed
From: Sven Eckelmann <sven@narfation.org>
To: b.a.t.m.a.n@lists.open-mesh.org
Cc: Sven Eckelmann <sven@narfation.org>
Subject: [B.A.T.M.A.N.] [PATCH 2/6] batctl: Force null termination of string after strncpy
Date: Sat, 24 May 2014 14:16:40 +0200	[thread overview]
Message-ID: <1400933804-9661-2-git-send-email-sven@narfation.org> (raw)
In-Reply-To: <1400933804-9661-1-git-send-email-sven@narfation.org>

strncpy doesn't terminate the string with a '\0' character when the length
of the destination memory location was shorter than the source string.
Accessing it again with string related functions isn't safe after such a
semi-failed copy and the caller has to handle it. The easiest way is to
always set the last character in the destination buffer to '\0' after the
strncpy was called.

Also the length provided as argument of strncpy should not be the length of
the source buffer but the maximum number of bytes in the destination buffer.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 bat-hosts.c | 6 ++++--
 bisect_iv.c | 2 ++
 debugfs.c   | 1 +
 functions.c | 8 ++++----
 tcpdump.c   | 2 ++
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/bat-hosts.c b/bat-hosts.c
index 30ff848..1d62bb8 100644
--- a/bat-hosts.c
+++ b/bat-hosts.c
@@ -108,7 +108,8 @@ static void parse_hosts_file(struct hashtable_t **hash, const char path[], int r
 			if (read_opt & USE_BAT_HOSTS)
 				fprintf(stderr, "Warning - mac already known (changing name from '%s' to '%s'): %s\n",
 					bat_host->name, name, mac_str);
-			strncpy(bat_host->name, name, HOST_NAME_MAX_LEN - 1);
+			strncpy(bat_host->name, name, HOST_NAME_MAX_LEN);
+			bat_host->name[HOST_NAME_MAX_LEN - 1] = '\0';
 			continue;
 		}
 
@@ -132,7 +133,8 @@ static void parse_hosts_file(struct hashtable_t **hash, const char path[], int r
 		}
 
 		memcpy(&bat_host->mac_addr, mac_addr, sizeof(struct ether_addr));
-		strncpy(bat_host->name, name, HOST_NAME_MAX_LEN - 1);
+		strncpy(bat_host->name, name, HOST_NAME_MAX_LEN);
+		bat_host->name[HOST_NAME_MAX_LEN - 1] = '\0';
 
 		hash_add(*hash, bat_host);
 
diff --git a/bisect_iv.c b/bisect_iv.c
index 0c25664..e9e7326 100644
--- a/bisect_iv.c
+++ b/bisect_iv.c
@@ -91,6 +91,7 @@ static struct bat_node *node_get(char *name)
 	}
 
 	strncpy(bat_node->name, name, NAME_LEN);
+	bat_node->name[NAME_LEN] = '\0';
 	INIT_LIST_HEAD_FIRST(bat_node->orig_event_list);
 	INIT_LIST_HEAD_FIRST(bat_node->rt_table_list);
 	memset(bat_node->loop_magic, 0, sizeof(bat_node->loop_magic));
@@ -1438,6 +1439,7 @@ static int get_orig_addr(char *orig_name, char *orig_addr)
 
 copy_name:
 	strncpy(orig_addr, orig_name_tmp, NAME_LEN);
+	orig_addr[NAME_LEN - 1] = '\0';
 	return 1;
 
 err:
diff --git a/debugfs.c b/debugfs.c
index 8033f8b..8dd78b1 100644
--- a/debugfs.c
+++ b/debugfs.c
@@ -149,6 +149,7 @@ char *debugfs_mount(const char *mountpoint)
 
 	/* save the mountpoint */
 	strncpy(debugfs_mountpoint, mountpoint, sizeof(debugfs_mountpoint));
+	debugfs_mountpoint[sizeof(debugfs_mountpoint) - 1] = '\0';
 	debugfs_found = 1;
 
 	return debugfs_mountpoint;
diff --git a/functions.c b/functions.c
index 0f96cb8..84f0d14 100644
--- a/functions.c
+++ b/functions.c
@@ -199,8 +199,8 @@ int read_file(const char *dir, const char *fname, int read_opt,
 	if (read_opt & USE_BAT_HOSTS)
 		bat_hosts_init(read_opt);
 
-	strncpy(full_path, dir, strlen(dir));
-	full_path[strlen(dir)] = '\0';
+	strncpy(full_path, dir, sizeof(full_path));
+	full_path[sizeof(full_path) - 1] = '\0';
 	strncat(full_path, fname, sizeof(full_path) - strlen(full_path) - 1);
 
 open:
@@ -339,8 +339,8 @@ int write_file(const char *dir, const char *fname, const char *arg1,
 	char full_path[500];
 	ssize_t write_len;
 
-	strncpy(full_path, dir, strlen(dir));
-	full_path[strlen(dir)] = '\0';
+	strncpy(full_path, dir, sizeof(full_path));
+	full_path[sizeof(full_path) - 1] = '\0';
 	strncat(full_path, fname, sizeof(full_path) - strlen(full_path) - 1);
 
 	fd = open(full_path, O_WRONLY);
diff --git a/tcpdump.c b/tcpdump.c
index 10b18f2..e84617e 100644
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -865,6 +865,7 @@ int tcpdump(int argc, char **argv)
 
 		memset(&req, 0, sizeof (struct ifreq));
 		strncpy(req.ifr_name, dump_if->dev, IFNAMSIZ);
+		req.ifr_name[sizeof(req.ifr_name) - 1] = '\0';
 
 		res = ioctl(dump_if->raw_sock, SIOCGIFHWADDR, &req);
 		if (res < 0) {
@@ -887,6 +888,7 @@ int tcpdump(int argc, char **argv)
 
 		memset(&req, 0, sizeof (struct ifreq));
 		strncpy(req.ifr_name, dump_if->dev, IFNAMSIZ);
+		req.ifr_name[sizeof(req.ifr_name) - 1] = '\0';
 
 		res = ioctl(dump_if->raw_sock, SIOCGIFINDEX, &req);
 
-- 
2.0.0.rc2


  reply	other threads:[~2014-05-24 12:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-24 12:16 [B.A.T.M.A.N.] [PATCH 1/6] batctl: Don't try to close negative file descriptors Sven Eckelmann
2014-05-24 12:16 ` Sven Eckelmann [this message]
2014-06-10 14:41   ` [B.A.T.M.A.N.] [PATCH 2/6] batctl: Force null termination of string after strncpy Marek Lindner
2014-05-24 12:16 ` [B.A.T.M.A.N.] [PATCH 3/6] batctl: Use strncpy instead of strcpy for string copy Sven Eckelmann
2014-06-10 14:49   ` Marek Lindner
2014-05-24 12:16 ` [B.A.T.M.A.N.] [PATCH 4/6] batctl: Return success only with valid line_ptr in read_file Sven Eckelmann
2014-06-10 14:53   ` Marek Lindner
2014-05-24 12:16 ` [B.A.T.M.A.N.] [PATCH 5/6] batctl: Initialize complete ping packet before write Sven Eckelmann
2014-06-10 14:58   ` Marek Lindner
2014-05-24 12:16 ` [B.A.T.M.A.N.] [PATCH 6/6] batctl: Don't provide uninitialized parameter to read_file Sven Eckelmann
2014-06-10 14:59   ` Marek Lindner
2014-06-10 14:31 ` [B.A.T.M.A.N.] [PATCH 1/6] batctl: Don't try to close negative file descriptors Marek Lindner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1400933804-9661-2-git-send-email-sven@narfation.org \
    --to=sven@narfation.org \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox