[PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[Peter Eriksen]

Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>
---

I've taken strlcpy() from the OpenBSD CVS without attribution.  Is this
allowed?  If it is, how should it be stated?

I think this fixes some small errors, but might introduce some new ones.
I've tried to be very careful, but this really needs some more eyeballs.
What do you think?

Peter

P.S. By the way, the diff of safe_strncpy() isn't so pretty, because
what I really did was replace the entire function, not edit it.

 builtin-log.c      |    2 +-
 builtin-tar-tree.c |    4 ++--
 cache.h            |    2 +-
 config.c           |    6 +++---
 http-fetch.c       |   10 ++++------
 http-push.c        |   10 +++++-----
 ident.c            |    5 ++---
 path.c             |   31 +++++++++++++++++++++++++++----
 sha1_name.c        |    3 +--
 9 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 29a8851..5b0ea28 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -112,7 +112,7 @@ static void reopen_stdout(struct commit 
 	int len = 0;
 
 	if (output_directory) {
-		strncpy(filename, output_directory, 1010);
+		safe_strncpy(filename, output_directory, 1010);
 		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 58a8ccd..f6310b9 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -240,8 +240,8 @@ static void write_entry(const unsigned c
 	/* XXX: should we provide more meaningful info here? */
 	sprintf(header.uid, "%07o", 0);
 	sprintf(header.gid, "%07o", 0);
-	strncpy(header.uname, "git", 31);
-	strncpy(header.gname, "git", 31);
+	safe_strncpy(header.uname, "git", sizeof(header.uname));
+	safe_strncpy(header.gname, "git", sizeof(header.gname));
 	sprintf(header.devmajor, "%07o", 0);
 	sprintf(header.devminor, "%07o", 0);
 
diff --git a/cache.h b/cache.h
index d5d7fe4..f630cf4 100644
--- a/cache.h
+++ b/cache.h
@@ -210,7 +210,7 @@ int git_mkstemp(char *path, size_t n, co
 
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
-char *safe_strncpy(char *, const char *, size_t);
+size_t safe_strncpy(char *, const char *, size_t);
 char *enter_repo(char *path, int strict);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
diff --git a/config.c b/config.c
index c474970..984c75f 100644
--- a/config.c
+++ b/config.c
@@ -280,17 +280,17 @@ int git_default_config(const char *var, 
 	}
 
 	if (!strcmp(var, "user.name")) {
-		strncpy(git_default_name, value, sizeof(git_default_name));
+		safe_strncpy(git_default_name, value, sizeof(git_default_name));
 		return 0;
 	}
 
 	if (!strcmp(var, "user.email")) {
-		strncpy(git_default_email, value, sizeof(git_default_email));
+		safe_strncpy(git_default_email, value, sizeof(git_default_email));
 		return 0;
 	}
 
 	if (!strcmp(var, "i18n.commitencoding")) {
-		strncpy(git_commit_encoding, value, sizeof(git_commit_encoding));
+		safe_strncpy(git_commit_encoding, value, sizeof(git_commit_encoding));
 		return 0;
 	}
 
diff --git a/http-fetch.c b/http-fetch.c
index d3602b7..da1a7f5 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -584,10 +584,8 @@ static void process_alternates_response(
 			// skip 'objects' at end
 			if (okay) {
 				target = xmalloc(serverlen + posn - i - 6);
-				strncpy(target, base, serverlen);
-				strncpy(target + serverlen, data + i,
-					posn - i - 7);
-				target[serverlen + posn - i - 7] = '\0';
+				safe_strncpy(target, base, serverlen);
+				safe_strncpy(target + serverlen, data + i, posn - i - 6);
 				if (get_verbosely)
 					fprintf(stderr,
 						"Also look at %s\n", target);
@@ -728,8 +726,8 @@ xml_cdata(void *userData, const XML_Char
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
 	if (ctx->cdata)
 		free(ctx->cdata);
-	ctx->cdata = xcalloc(len+1, 1);
-	strncpy(ctx->cdata, s, len);
+	ctx->cdata = xmalloc(len + 1);
+	safe_strncpy(ctx->cdata, s, len + 1);
 }
 
 static int remote_ls(struct alt_base *repo, const char *path, int flags,
diff --git a/http-push.c b/http-push.c
index b39b36b..2d9441e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1269,8 +1269,8 @@ xml_cdata(void *userData, const XML_Char
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
 	if (ctx->cdata)
 		free(ctx->cdata);
-	ctx->cdata = xcalloc(len+1, 1);
-	strncpy(ctx->cdata, s, len);
+	ctx->cdata = xmalloc(len + 1);
+	safe_strncpy(ctx->cdata, s, len + 1);
 }
 
 static struct remote_lock *lock_remote(char *path, long timeout)
@@ -1472,7 +1472,7 @@ static void process_ls_object(struct rem
 		return;
 	path += 8;
 	obj_hex = xmalloc(strlen(path));
-	strncpy(obj_hex, path, 2);
+	safe_strncpy(obj_hex, path, 3);
 	strcpy(obj_hex + 2, path + 3);
 	one_remote_object(obj_hex);
 	free(obj_hex);
@@ -2160,8 +2160,8 @@ static void fetch_symref(char *path, cha
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
 	if (!strncmp((char *)buffer.buffer, "ref: ", 5)) {
-		*symref = xcalloc(buffer.posn - 5, 1);
-		strncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 6);
+		*symref = xmalloc(buffer.posn - 5);
+		safe_strncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
 	} else {
 		get_sha1_hex(buffer.buffer, sha1);
 	}
diff --git a/ident.c b/ident.c
index 7c81fe8..7b44cbd 100644
--- a/ident.c
+++ b/ident.c
@@ -71,10 +71,9 @@ int setup_ident(void)
 		len = strlen(git_default_email);
 		git_default_email[len++] = '.';
 		if (he && (domainname = strchr(he->h_name, '.')))
-			strncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
+			safe_strncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
 		else
-			strncpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
-		git_default_email[sizeof(git_default_email) - 1] = 0;
+			safe_strncpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
 	}
 	/* And set the default date */
 	datestamp(git_default_date, sizeof(git_default_date));
diff --git a/path.c b/path.c
index 5168b5f..86f51e0 100644
--- a/path.c
+++ b/path.c
@@ -83,14 +83,37 @@ int git_mkstemp(char *path, size_t len, 
 }
 
 
-char *safe_strncpy(char *dest, const char *src, size_t n)
+/*
+ * Copy src to string dst of size siz.  At most siz-1 characters
+ * will be copied.  Always NUL terminates (unless siz == 0).
+ * Returns strlen(src); if retval >= siz, truncation occurred.
+ */
+size_t safe_strncpy(char *dst, const char *src, size_t siz)
 {
-	strncpy(dest, src, n);
-	dest[n - 1] = '\0';
+	char *d = dst;
+	const char *s = src;
+	size_t n = siz;
 
-	return dest;
+	/* Copy as many bytes as will fit */
+	if (n != 0) {
+		while (--n != 0) {
+			if ((*d++ = *s++) == '\0')
+				break;
+		}
+	}
+
+	/* Not enough room in dst, add NUL and traverse rest of src */
+	if (n == 0) {
+		if (siz != 0)
+			*d = '\0';		/* NUL-terminate dst */
+		while (*s++)
+			;
+	}
+
+	return(s - src - 1);	/* count does not include NUL */
 }
 
+
 int validate_symref(const char *path)
 {
 	struct stat st;
diff --git a/sha1_name.c b/sha1_name.c
index fbbde1c..8fe9b7a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -262,8 +262,7 @@ static int get_sha1_basic(const char *st
 		if (str[am] == '@' && str[am+1] == '{' && str[len-1] == '}') {
 			int date_len = len - am - 3;
 			char *date_spec = xmalloc(date_len + 1);
-			strncpy(date_spec, str + am + 2, date_len);
-			date_spec[date_len] = 0;
+			safe_strncpy(date_spec, str + am + 2, date_len + 1);
 			at_time = approxidate(date_spec);
 			free(date_spec);
 			len = am;
-- 
1.3.3.g16a4

Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20060611100628.GA10430@bohr.gbar.dtu.dk> (at Sun, 11 Jun 2006 12:06:28 +0200), "Peter Eriksen" <s022018@student.dtu.dk> says:

> I've taken strlcpy() from the OpenBSD CVS without attribution.  Is this
> allowed?  If it is, how should it be stated?

Please include full copyright information.

--yoshfuji

Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[Peter Eriksen]

On Sun, Jun 11, 2006 at 07:15:40PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> In article <20060611100628.GA10430@bohr.gbar.dtu.dk> (at Sun, 11 Jun 2006 12:06:28 +0200), "Peter Eriksen" <s022018@student.dtu.dk> says:
> 
> > I've taken strlcpy() from the OpenBSD CVS without attribution.  Is this
> > allowed?  If it is, how should it be stated?
> 
> Please include full copyright information.

Where should this information go?  Just above the function
safe_strncpy(), or at the top of path.c?  I believe path.c is GPL, so
can this be mixed freely with BSD licensed code?  Should I put
safe_strncpy() into a seperate file as with strlcpy()?
This seems to be the copyright information:

/*
 * Copyright (c) 1998 Todd C. Miller <Todd.Miller@courtesan.com>
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

Peter

Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[Rene Scharfe]

Peter Eriksen schrieb:
> On Sun, Jun 11, 2006 at 07:15:40PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>> In article <20060611100628.GA10430@bohr.gbar.dtu.dk> (at Sun, 11 Jun 2006 12:06:28 +0200), "Peter Eriksen" <s022018@student.dtu.dk> says:
>>
>>> I've taken strlcpy() from the OpenBSD CVS without attribution.  Is this
>>> allowed?  If it is, how should it be stated?
>> Please include full copyright information.
> 
> Where should this information go?  Just above the function
> safe_strncpy(), or at the top of path.c?  I believe path.c is GPL, so
> can this be mixed freely with BSD licensed code?  Should I put
> safe_strncpy() into a seperate file as with strlcpy()?

Yes...  Or you could avoid all of this by using a GPL'd version, like
the one from the Linux kernel (in lib/string.c).

René

Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more.

[Linus Torvalds]

On Sun, 11 Jun 2006, Peter Eriksen wrote:
> > 
> > Please include full copyright information.
> 
> Where should this information go?

Gaah, for soemthing as trivial as strlcpy(), please either just rewrite 
the function (you can only do it in so many ways), since the copyright 
message is _bigger_ and more annoying than the code itself.

Or, just attribute it. Say "this is a trivial implementation 'strlcpy()' 
shamelessly stolen from OpenBSD, originally under a BSD license, converted 
to GPLv2"

That whole "big copyright messages" thign is a _disease_. 

		Linus