git/path.c, branch v2.16.2

Merge branch 'ao/path-use-xmalloc'

2017-10-28T01:18:40Z

A possible oom error is now caught as a fatal error, instead of continuing and dereferencing NULL. * ao/path-use-xmalloc: path.c: use xmalloc() in add_to_trie()

path.c: use xmalloc() in add_to_trie()

2017-10-25T07:16:22Z

Add usage of xmalloc() instead of malloc() in add_to_trie() as xmalloc wraps and checks memory allocation result. Signed-off-by: Andrey Okoshkin Signed-off-by: Junio C Hamano

Merge branch 'tg/memfixes'

2017-10-07T07:27:54Z

Fixes for a handful memory access issues identified by valgrind. * tg/memfixes: sub-process: use child_process.args instead of child_process.argv http-push: fix construction of hex value from path path.c: fix uninitialized memory access

Merge branch 'rs/cleanup-strbuf-users'

2017-10-05T04:48:19Z

Code clean-up. * rs/cleanup-strbuf-users: graph: use strbuf_addchars() to add spaces use strbuf_addstr() for adding strings to strbufs path: use strbuf_add_real_path()

path.c: fix uninitialized memory access

2017-10-04T04:47:16Z

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 Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano

Merge branch 'jk/validate-headref-fix'

2017-10-03T06:42:49Z

Code clean-up. * jk/validate-headref-fix: validate_headref: use get_oid_hex for detached HEADs validate_headref: use skip_prefix for symref parsing validate_headref: NUL-terminate HEAD buffer

path: use strbuf_add_real_path()

2017-10-02T04:13:41Z

Avoid a string copy to a static buffer by using strbuf_add_real_path() instead of combining strbuf_addstr() and real_path(). Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano

validate_headref: use get_oid_hex for detached HEADs

2017-09-27T07:07:22Z

If a candidate HEAD isn't a symref, we check that it contains a viable sha1. But in a post-sha1 world, we should be checking whether it has any plausible object-id. We can do that by switching to get_oid_hex(). Note that both before and after this patch, we only check for a plausible object id at the start of the file, and then call that good enough. We ignore any content _after_ the hex, so a string like: 0123456789012345678901234567890123456789 foo is accepted. Though we do put extra bytes like this into some pseudorefs (e.g., FETCH_HEAD), we don't typically do so with HEAD. We could tighten this up by using parse_oid_hex(), like: if (!parse_oid_hex(buffer, &oid, &end) && *end++ == '\n' && *end == '\0') return 0; But we're probably better to remain on the loose side. We're just checking here for a plausible-looking repository directory, so heuristics are acceptable (if we really want to be meticulous, we should use the actual ref code to parse HEAD). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

validate_headref: use skip_prefix for symref parsing

2017-09-27T07:06:31Z

Since the previous commit guarantees that our symref buffer is NUL-terminated, we can just use skip_prefix() and friends to parse it. This is shorter and saves us having to deal with magic numbers and keeping the "len" counter up to date. While we're at it, let's name the rather obscure "buf" to "refname", since that is the thing we are parsing with it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

validate_headref: NUL-terminate HEAD buffer

2017-09-27T07:01:24Z

When we are checking to see if we have a git repo, we peek into the HEAD file and see if it's a plausible symlink, symref, or detached HEAD. For the latter two, we read the contents with read_in_full(), which means they aren't NUL-terminated. The symref check is careful to respect the length we got, but the sha1 check will happily parse up to 40 bytes, even if we read fewer. E.g.,: echo 1234 >.git/HEAD git rev-parse will parse 36 uninitialized bytes from our stack buffer. This isn't a big deal in practice. Our buffer is 256 bytes, so we know we'll never read outside of it. The worst case is that the uninitialized bytes look like valid hex, and we claim a bogus HEAD file is valid. The chances of this happening randomly are quite slim, but let's be careful. One option would be to check that "len == 41" before feeding the buffer to get_sha1_hex(). But we'd like to eventually prepare for a world with variable-length hashes. Let's NUL-terminate as soon as we've read the buffer (we already even leave a spare byte to do so!). That fixes this problem without depending on the size of an object id. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano