Thread (18 messages) 18 messages, 6 authors, 2017-10-05

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

From: Jonathan Nieder <hidden>
Date: 2017-10-03 22:45:10

Hi,

Thomas Gummerer wrote:
In cleanup_path we're passing in a char array, run a memcmp on it, and
run through it without ever checking if something is in the array in the
first place.  This can lead us to access uninitialized memory, for
example in t5541-http-push-smart.sh test 7, when run under valgrind:
[...]
Avoid this by checking passing in the length of the string in the char
array, and checking that we never run over it.

Signed-off-by: Thomas Gummerer <redacted>
---
 path.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
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?

Thanks and hope that helps,
Jonathan
diff --git i/path.c w/path.c
index b533ec938d..3a1fbee1e0 100644
--- i/path.c
+++ w/path.c
@@ -37,7 +37,7 @@ static struct strbuf *get_pathname(void)
 static char *cleanup_path(char *path)
 {
 	/* Clean it up */
-	if (!memcmp(path, "./", 2)) {
+	if (*path == '.' && path[1] == '/') {
 		path += 2;
 		while (*path == '/')
 			path++;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help