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

[Jonathan Nieder]

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 <t.gumme...@gmail.com>
> ---
>  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++;