summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2017-10-03 19:30:40 -0400
committerJunio C Hamano <gitster@pobox.com>2017-10-04 13:47:16 +0900
commit8262715b8eb157497ec1ee1cfcef778d526b2336 (patch)
tree3acae7272c5fdf575d562ec0388bd9573b3b8abc
parent4010f1d1b782eb7585e0e0abcefa794bd5ff29a0 (diff)
downloadgit-8262715b8eb157497ec1ee1cfcef778d526b2336.tar.gz
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 <t.gummerer@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--path.c9
1 files changed, 4 insertions, 5 deletions
diff --git a/path.c b/path.c
index e50d2befcf..2fecf854fe 100644
--- a/path.c
+++ b/path.c
@@ -33,11 +33,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++;
}
@@ -46,7 +45,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);
}
@@ -63,7 +62,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)