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++;