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.
I'm seeing the current fuzzer build fail (during `cmake`) like so:
```
-- Performing Test IS_FSANITIZE_FUZZER_NO_LINK_SUPPORTED
-- Performing Test IS_FSANITIZE_FUZZER_NO_LINK_SUPPORTED - Failed
CMake Error at cmake/AddCFlagIfSupported.cmake:17 (message):
Required flag -fsanitize=fuzzer-no-link is not supported
Call Stack (most recent call first):
fuzzers/CMakeLists.txt:6 (add_c_flag)
```
The cmake log output contains something like so:
```
/src/aflplusplus/libAFLDriver.a(aflpp_driver.o): in function `main':
aflpp_driver.c:(.text+0x11b): undefined reference to `LLVMFuzzerTestOneInput'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
I haven't figured out exactly what's happening, but I believe that
once line 5 has added `-fsanitize=fuzzer` to `CFLAGS`, future compile-
tests **also** use it during linking. This in turn pulls in the fuzzer
`main`, which expects an `LLVMFuzzerTestOneInput` symbol, and thus
fails.
Instead, just add `-fsanitize=fuzzer-no-link` to CFLAGS (as suggested
[by the documentation][libfuzzer]), and then use `-fsanitize=fuzzer`
only for linking the fuzzer targets. At least in my environment, this
results in a working fuzzer build.
[libfuzzer]: https://llvm.org/docs/LibFuzzer.html#fuzzer-usage
Currently, the DefaultCFlags.cmake overrides the
CMAKE_STATIC_LINKER_FLAGS to suppress linker warnings about files with
no symbols defined.
This has the side effect of breaking MSVC cross compilation (where
CMAKE_STATIC_LINKER_FLAGS is used to specify the /MACHINE:ARCH flag)
This commit make sure we append to CMAKE_STATIC_LINKER_FLAGS instead of
replacing its values
I tried to build my libgit2-1.9.0 package for CYGWIN but I got an error. This message appears when compiling:
[199/671] Building C object src/cli/CMakeFiles/git2_cli.dir/opt.c.o
libgit2-1.9.0/src/cli/opt.c: In function ‘cli_opt_parse’:
libgit2-1.9.0/src/cli/opt.c:564:23: warning: implicit declaration of function ‘alloca’; did you mean ‘malloc’? [-Wimplicit-function-declaration]
564 | given_specs = alloca(sizeof(const cli_opt_spec *) * (args_len + 1));
| ^~~~~~
| malloc
and later the linker emits this error message:
[668/671] Linking C executable git2.exe
FAILED: git2.exe
/usr/x86_64-pc-cygwin/bin/ld: src/cli/CMakeFiles/git2_cli.dir/opt.c.o: in function `cli_opt_parse':
/usr/src/debug/libgit2-1.9.0-1/src/cli/opt.c:564:(.text+0xce3): undefined reference to `alloca'
collect2: error: ld returned 1 exit status
The error is fixed by adding alloca.h to included headers.
Hopefully, opt.c already allows to add alloca.h for some platforms, so I just added an additional test for the preprocessor for checking if the target is CYGWIN.
Don't build docs on pushes to maint branches; those docs should only be
built _on release_. In addition, be safer about not creating an existing
branch from a tracking branch.