[PATCH 1/3] path.c: fix uninitialized memory access
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Oct 3, 2017, 19:57 UTC
- Message-ID
- <20171003195713.13395-2-t.gummerer@gmail.com>
- In-Reply-To
- <20171003195713.13395-1-t.gummerer@gmail.com>
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 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.gummerer@gmail.com> --- path.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/path.c b/path.c index b533ec938d..12f41ee877 100644 --- a/path.c +++ b/path.c @@ -34,20 +34,21 @@ static struct strbuf *get_pathname(void) return sb; } -static char *cleanup_path(char *path) +static char *cleanup_path(char *path, int len) { /* Clean it up */ - if (!memcmp(path, "./", 2)) { - path += 2; - while (*path == '/') - path++; + int skip = 0; + if (len >= 2 && !memcmp(path, "./", 2)) { + skip += 2; + while (skip < len && *(path + skip) == '/') + skip++; } - return path; + return path + skip; } static void strbuf_cleanup_path(struct strbuf *sb) { - char *path = cleanup_path(sb->buf); + char *path = cleanup_path(sb->buf, sb->len); if (path > sb->buf) strbuf_remove(sb, 0, path - sb->buf); } @@ -64,7 +65,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...) strlcpy(buf, bad_path, n); return buf; } - return cleanup_path(buf); + return cleanup_path(buf, n); } static int dir_prefix(const char *buf, const char *dir) @@ -494,7 +495,7 @@ const char *mkpath(const char *fmt, ...) va_start(args, fmt); strbuf_vaddf(pathname, fmt, args); va_end(args); - return cleanup_path(pathname->buf); + return cleanup_path(pathname->buf, pathname->len); } const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
-- 2.14.1.480.gb18f417b89