@matheusbsbs / matheus-bagliano / commits / 8262715b8e

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>

Jeff King committed Oct 3, 2017 at 19:30 UTC 8262715b8eb157497ec1ee1cfcef778d526b2336

1 file changed +4 -5

path.c

+4 -5

@@ -33,11 +33,10 @@ static struct strbuf *get_pathname(void)

33 return sb;

34 }

36 -static char *cleanup_path(char *path)

36 +static const char *cleanup_path(const char *path)

37 {

38 /* Clean it up */

39 - if (!memcmp(path, "./", 2)) {

40 - path += 2;

39 + if (skip_prefix(path, "./", &path)) {

40 while (*path == '/')

41 path++;

42 }

@@ -46,7 +45,7 @@ static char *cleanup_path(char *path)

46 static void strbuf_cleanup_path(struct strbuf *sb)

47 {

49 - char *path = cleanup_path(sb->buf);

48 + const char *path = cleanup_path(sb->buf);

49 if (path > sb->buf)

50 strbuf_remove(sb, 0, path - sb->buf);

51 }

@@ -63,7 +62,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)

62 strlcpy(buf, bad_path, n);

63 return buf;

64 }

66 - return cleanup_path(buf);

65 + return (char *)cleanup_path(buf);

66 }

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