With a recent upgrade to a newer version of MSVC we now get a bunch of
warnings when two operands use different enum types. While sensible in
theory, in practice we have a couple of non-public enums that extend
public enums, like for example with `GIT_SUBMODULE_STATUS`.
Let's for now disable this warning to unblock our builds. The
alternative would be to add casts all over the place, but that feels
rather cumbersome.
Look for the `GIT_SSH_COMMAND` environment variable and prefer it to
`GIT_SSH`. The `GIT_SSH_COMMAND` will execute via the shell, which is
useful to provide additional arguments.
When sending paths to the remote server, escape them properly.
Escape them with a single quote, followed by the escaped character,
followed by another single quote. This prevents misparsing on the
remote side and potential command injection.
Allow `git_str_puts_escaped` to take an escaping prefix and an escaping
suffix; this allows for more options, including the ability to better
support escaping executed paths.
Construct the arguments for the ssh exec as an explicit array, instead
of trying to create a command-line for sh. The latter may use user input
(the remote path) so this may be vulnerable to command injection.
When using `git_process_new` on win32, resolve the path to the
application in the same way that we do on POSIX.
Search `PATH` for command to execute (unless the given executable is
fully qualified). In addition, better match Windows executable lookup
behavior itself (allowing the command to be `foo`, and looking for a
matching `foo.exe` or `foo.cmd`.)
By default, `git_process_new` will no longer try to prepare a single
string to execute with the shell. Instead, by default, arguments remain
parameterized and the command to execute is located within the `PATH`.
The shell can also still optionally be used (so that additional
arguments can be included and variables handled appropriately) but this
is done by keeping arguments parameterized for safety.
This new behavior prevents accidental misuse and potential command-line
injection.
Ensure that when we look for an executable on Windows that we add
executable suffixes (`.exe`, `.cmd`). Without this, we would not support
looking for (eg) `ssh`, since we actually need to identify a file named
`ssh.exe` (or `ssh.cmd`) in `PATH`.
* Do not search `PATH` for fully- or partially-qualified filenames
(eg, `foo/bar`)
* Ensure that a file in the `PATH` is executable before returning it
Ensure that our `find_executable` behaves as expected:
* When the executable contains a fully- or partially-qualified filename
component (eg, `foo/bar`) that `PATH` is not searched; these paths are
relative to the current working directory.
* An empty segment in `PATH` (on POSIX systems) is treated as the
current directory; this is for compatibility with Bourne shells.
* When a file exists in `PATH`, it is actually executable (on POSIX)
The `ssh_custom_free()` function calls `strlen()` on the `publickey`
field, which stores binary data, not a null-terminated string. This
causes a heap buffer overflow when the public key data is not
null-terminated or contains embedded null bytes.
The `publickey` field stores binary data, as required by the underlying
`libssh2_userauth_publickey()` function, which accepts a public key
parameter of the type `const unsigned char*`.
Use the stored `publickey_len` instead of `strlen()` to determine the
correct buffer size.
Fixes a bug likely introduced in
d396819101 (in 1.8.1) whereby
‘proxy_settings.on_status’ would be left uninitialized when using the
‘http-parser’ backend, eventually leading to a segfault in
‘http_parser_execute’. Valgrind would report use of the uninitialized
value like so:
Conditional jump or move depends on uninitialised value(s)
at 0x50CD533: http_parser_execute (http_parser.c:910)
by 0x4928504: git_http_parser_execute (httpparser.c:82)
by 0x4925C42: client_read_and_parse (httpclient.c:1178)
by 0x4926F27: git_http_client_read_response (httpclient.c:1458)
by 0x49255FE: http_stream_read (http.c:427)
by 0x4929B90: git_smart__recv (smart.c:29)
by 0x492C147: git_smart__store_refs (smart_protocol.c:58)
by 0x4929F6C: git_smart__connect (smart.c:171)
by 0x4904DCE: git_remote_connect_ext (remote.c:963)
by 0x48A15D2: clone_into (clone.c:449)
by 0x48A15D2: git__clone (clone.c:546)
by 0x4010E9: main (libgit2-proxy.c:20)
Description: In an older version of libgit2 in git_object_lookup_prefix was a check that repo is valid, but now there is repo->oid_type in the git_object_lookup and must be checked as well
realpath(3) _may_ allocate strings (if the second param is NULL) using
the system allocator. However, callers need an assurance that they can
free memory using git__free. If we made realpath do an allocation, then
make sure that we strdup it into our allocator's memory.
More importantly, avoid this behavior by always providing a buffer to
p_realpath invocations.
Instead of tweaking the `stdalloc` allocator when
`GIT_DEBUG_STRICT_ALLOC` is defined, actually create a debugging
allocator. This allows us to ensure that we are strict about things like
not expecting `malloc(0)` to do something useful, but we can also
introduce an excessively pedantic `realloc` implementation that _always_
creates a new buffer, throws away its original `ptr`, and overwrites the
data that's there with garbage. This may be helpful to identify places
that make assumptions about realloc.
Avoid sloppy aliasing in our (re-)allocation, which is undefined
behavior. This has been problematic before and was helped by `volatile`
(see b62a6a13b2) but that is not
technically correct, and some compilers / architectures do not
understand that `ptr` is changing due to its aliasing.
Just make `git_array_alloc` behave like `realloc`, taking a `void *` and
returning a `void *`.
