Skip to content

Commit 8262715

Browse files
peffgitster
authored andcommitted
path.c: fix uninitialized memory access
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: ==4423== Conditional jump or move depends on uninitialised value(s) ==4423== at 0x242FA9: cleanup_path (path.c:35) ==4423== by 0x242FA9: mkpath (path.c:456) ==4423== by 0x256CC7: refname_match (refs.c:364) ==4423== by 0x26C181: count_refspec_match (remote.c:1015) ==4423== by 0x26C181: match_explicit_lhs (remote.c:1126) ==4423== by 0x26C181: check_push_refs (remote.c:1409) ==4423== by 0x2ABB4D: transport_push (transport.c:870) ==4423== by 0x186703: push_with_options (push.c:332) ==4423== by 0x18746D: do_push (push.c:409) ==4423== by 0x18746D: cmd_push (push.c:566) ==4423== by 0x1183E0: run_builtin (git.c:352) ==4423== by 0x11973E: handle_builtin (git.c:539) ==4423== by 0x11973E: run_argv (git.c:593) ==4423== by 0x11973E: main (git.c:698) ==4423== Uninitialised value was created by a heap allocation ==4423== at 0x4C2CD8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==4423== by 0x4C2F195: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==4423== by 0x2C196B: xrealloc (wrapper.c:137) ==4423== by 0x29A30B: strbuf_grow (strbuf.c:66) ==4423== by 0x29A30B: strbuf_vaddf (strbuf.c:277) ==4423== by 0x242F9F: mkpath (path.c:454) ==4423== by 0x256CC7: refname_match (refs.c:364) ==4423== by 0x26C181: count_refspec_match (remote.c:1015) ==4423== by 0x26C181: match_explicit_lhs (remote.c:1126) ==4423== by 0x26C181: check_push_refs (remote.c:1409) ==4423== by 0x2ABB4D: transport_push (transport.c:870) ==4423== by 0x186703: push_with_options (push.c:332) ==4423== by 0x18746D: do_push (push.c:409) ==4423== by 0x18746D: cmd_push (push.c:566) ==4423== by 0x1183E0: run_builtin (git.c:352) ==4423== by 0x11973E: handle_builtin (git.c:539) ==4423== by 0x11973E: run_argv (git.c:593) ==4423== by 0x11973E: main (git.c:698) ==4423== Avoid this by using skip_prefix(), which knows not to go beyond the end of the string. Reported-by: Thomas Gummerer <[email protected]> Signed-off-by: Jeff King <[email protected]> Reviewed-by: Jonathan Nieder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4010f1d commit 8262715

File tree

1 file changed

+4
-5
lines changed

1 file changed

+4
-5
lines changed

path.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ static struct strbuf *get_pathname(void)
3333
return sb;
3434
}
3535

36-
static char *cleanup_path(char *path)
36+
static const char *cleanup_path(const char *path)
3737
{
3838
/* Clean it up */
39-
if (!memcmp(path, "./", 2)) {
40-
path += 2;
39+
if (skip_prefix(path, "./", &path)) {
4140
while (*path == '/')
4241
path++;
4342
}
@@ -46,7 +45,7 @@ static char *cleanup_path(char *path)
4645

4746
static void strbuf_cleanup_path(struct strbuf *sb)
4847
{
49-
char *path = cleanup_path(sb->buf);
48+
const char *path = cleanup_path(sb->buf);
5049
if (path > sb->buf)
5150
strbuf_remove(sb, 0, path - sb->buf);
5251
}
@@ -63,7 +62,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
6362
strlcpy(buf, bad_path, n);
6463
return buf;
6564
}
66-
return cleanup_path(buf);
65+
return (char *)cleanup_path(buf);
6766
}
6867

6968
static int dir_prefix(const char *buf, const char *dir)

0 commit comments

Comments
 (0)