Web lists-archives.com

Re: [PATCH 1/3] path.c: fix uninitialized memory access




On Tue, Oct 03, 2017 at 03:45:01PM -0700, Jonathan Nieder wrote:

> When I first read the above, I thought it was going to be about a
> NUL-terminated string that was missing a NUL.  But in fact, the issue
> is that strlen(path) can be < 2.
> 
> In other words, an alternative fix would be
> 
> 	if (*path == '.' && path[1] == '/') {
> 		...
> 	}
> 
> which would not require passing in 'len' or switching to index-based
> arithmetic.  I think I prefer it.  What do you think?

Yes, I think that approach is much nicer. I think you could even use
skip_prefix. Unfortunately you have to play a few games with const-ness,
but I think the resulting signature for cleanup_path() is an
improvement:

diff --git a/path.c b/path.c
index 00ec04e7a5..2e09a7bce0 100644
--- a/path.c
+++ b/path.c
@@ -34,11 +34,10 @@ static struct strbuf *get_pathname(void)
 	return sb;
 }
 
-static char *cleanup_path(char *path)
+static const char *cleanup_path(const char *path)
 {
 	/* Clean it up */
-	if (!memcmp(path, "./", 2)) {
-		path += 2;
+	if (skip_prefix(path, "./", &path)) {
 		while (*path == '/')
 			path++;
 	}
@@ -47,7 +46,7 @@ static char *cleanup_path(char *path)
 
 static void strbuf_cleanup_path(struct strbuf *sb)
 {
-	char *path = cleanup_path(sb->buf);
+	const char *path = cleanup_path(sb->buf);
 	if (path > sb->buf)
 		strbuf_remove(sb, 0, path - sb->buf);
 }
@@ -64,7 +63,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 		strlcpy(buf, bad_path, n);
 		return buf;
 	}
-	return cleanup_path(buf);
+	return (char *)cleanup_path(buf);
 }
 
 static int dir_prefix(const char *buf, const char *dir)