The implementation here seems to be sort of a copy
from the reference impl in RFC 6234 [2].
When multiple threads hash concurrently,
they race on this shared static variable.
It then corrupts the length-overflow detection,
and produces incorrect SHA-256 digests.
Here we replace it with a `static` function with a local variable.
The bug only affects the `GIT_SHA256_BUILTIN` backend.
The SHA-1 code path uses `sha1dc` which does not have this issue.
Reproducer:
```c
#include <stddef.h>
#include <stdio.h>
#include <string.h>
#include <pthread.h>
#include <git2.h>
#define NUM_THREADS 8
#define ITERATIONS 100000
static volatile int found_bug = 0;
void *hash_thread(void *arg) {
int id = *(int *)arg;
const char *data = "hello world\n";
size_t len = strlen(data);
git_object_id_options opts = GIT_OBJECT_ID_OPTIONS_INIT;
opts.object_type = GIT_OBJECT_BLOB;
opts.oid_type = GIT_OID_SHA256;
git_oid reference, result;
git_object_id_from_buffer(&reference, data, len, &opts);
for (int i = 0; i < ITERATIONS && !found_bug; i++) {
git_object_id_from_buffer(&result, data, len, &opts);
if (!git_oid_equal(&reference, &result)) {
found_bug = 1;
printf("BUG: thread %d, iteration %d\n", id, i);
break;
}
}
return NULL;
}
int main(void) {
git_libgit2_init();
pthread_t threads[NUM_THREADS];
int ids[NUM_THREADS];
for (int i = 0; i < NUM_THREADS; i++) {
ids[i] = i;
pthread_create(&threads[i], NULL, hash_thread, &ids[i]);
}
for (int i = 0; i < NUM_THREADS; i++)
pthread_join(threads[i], NULL);
if (!found_bug)
printf("No bug triggered\n");
git_libgit2_shutdown();
return found_bug ? 1 : 0;
}
```
Build and run (from libgit2 repo root):
```sh
mkdir build && cd build
cmake .. -DEXPERIMENTAL_SHA256=ON -DUSE_SHA256=Builtin \
-DUSE_HTTPS=OFF -DUSE_SSH=OFF -DUSE_NTLMCLIENT=OFF \
-DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Debug
make libgit2package
cd ..
cc -O0 -pthread -DGIT_EXPERIMENTAL_SHA256=1 \
-I include -o repro repro.c \
build/libgit2-experimental.a -lz -lpcre2-8
./repro
```
See <https://github.com/rust-lang/git2-rs/issues/1255> for more.
[1]: https://github.com/libgit2/libgit2/blob/1affb8b19/src/util/hash/rfc6234/sha224-256.c#L86-L91
[2]: https://www.rfc-editor.org/rfc/rfc6234#section-8.2.2
Make a distinction between generated headers and "translated" headers.
This is important to support build-time dependencies when headers are
updated.
Generated headers are those which contain build-time feature
specifications, like `git2_features.h` that are internal to the build
and `experimental.h` that contain API information.
Translated headers are the headers that are in `include/git2`, but may
be translated to have a unique prefix like `incklude/git2-experimental`.
This distinction is important so that the CMakeFiles.txt depend on the
in-tree include files (`src/include`) and the generated header files
_but not_ the translated header files. Otherwise there are two `pack.h`
and it's unclear whether the in-tree build is targeting the one in
`src/include` or the one in the build tree.
Without this, updating an in-tree header file like `pack.h` would not
cause a rebuild of its dependencies.
Git supports relative worktrees since Git v2.48 - cf6f63ea6b/Documentation/RelNotes/2.48.0.adoc (L57)
This was already handled programatically in libgit2, but was
not recognized as an extension, meaning downstream consumers
like Nix had issues with relative worktree-enabled repos.
Fixes#7210
In cases when a path is missing from the ancestor and only of the two
children is defined (IOW, the object is added), then the path in the child
that is defined should be returned.
However, merge_file__best_path is only returning a path in cases when there
is no ancestor path only if both children are defined and they match each
other.
Improve merge_file__best_path by handling the case when it is present in
only one of the children paths returning this value.
Depending on the zlib library used, the inflate() function may write
beyond the object size into the additional trailing buffer for aligned
memory copies. This may cause out-of-bounds memory read if the object
buffer is later used without checking the object size, as in
git_commit__extract_signature.
Link: https://github.com/zlib-ng/zlib-ng/issues/1767
Signed-off-by: Kan-Ru Chen <kanru@kanru.info>
Avoid a buffer overrun when an object's header specifies a short length
but the body is longer (fix#7177). Take care to preserve the behavior
that too-short object bodies are not an error but get zero-padded.
Instead of regenerating `git2.h` on every cmake invocation, use
`configure_file` to avoid rewriting it. This keeps timestamps inline and
avoids unnecessarily rebuilding the library.
The strstr call used to find the " object-format=" capability string did
not have a length limit, potentially reading past the end of the
allocated buffer if the capabilities string was not null-terminated
within the buffer bounds. Replaced strstr with git__memmem and
subsequent strchr calls with memchr, providing the remaining buffer
length as a limit to prevent out-of-bounds reads.
https: //oss-fuzz.com/testcase-detail/4895812384325632
https: //issues.oss-fuzz.com/issues/42524461
Change-Id: Id313af1bce48ea8763fa2dfd7eb9ee8934fa541f
Cache `oid_type` in `transport_local` struct during `connect()`
so `git_remote_oid_type()` keeps working after disconnect.
This matches the smart transport behavior.
Exercise git_remote_oid_type on a SHA256 local transport.
The after-disconnect assertion is commented out because
local_oid_type dereferences `t->repo`, which local_close
has already freed (SIGSEGV).
I ran these commands to test btw:
```
cmake .. -DGIT_EXPERIMENTAL_SHA256=ON -DBUILD_TESTS=ON
cmake --build .
./libgit2_tests -snetwork::remote::local::sha256_oid_type
```
Detached remotes already read global/system config for http proxy
settings, but did not apply url.*.insteadOf or url.*.pushInsteadOf
rules. This inconsistency meant that `git_remote_create_detached`
behaved differently from git's `ls-remote`, which was the primary
use case for detached remotes.
This fixes it by loading the default config when no repository is
provided and apply insteadOf rules consistently.
While this is a behavior change, it still respects
`GIT_REMOTE_CREATE_SKIP_INSTEADOF`, meaning that user can restore
the previous behavior with minimal effort
Fixes https://github.com/libgit2/libgit2/issues/5469
`git_remote_create_detached` does not apply
`url.*.insteadOf` or url.*.pushInsteadOf from global config.
These tests currently pass, asserting the buggy behavior
and serving as a minimal reproduction.
The assertions will be updated when the fix is applied.
See <https://github.com/libgit2/libgit2/issues/5469>
`clone_local_into()` copies objects from the source repository
but does not set the destination's object format.
This cause SHA256 repositories to be unreadable after a local clone
because the destination defaults to SHA1.
The remote clone path already handles this correctly
so local clone path follows suite.
See: <https://github.com/libgit2/libgit2/blob/1f34e2a57/src/libgit2/clone.c#L448-L450>
Local clone, both hardlink and copy paths, does not propagate
the source repository's object format to the destination.
A SHA256 repository cloned locally is initialized as SHA1
and caused object lookup failures.
These tests are added as reproducible examples,
and will be fixed in the next commit
* `sha256_via_no_local`
* `file://` URL + `GIT_CLONE_NO_LOCAL` -> remote code path
* `sha256_object_format_is_propagated`
* `file://` URL + `GIT_CLONE_LOCAL` -> local code path with hardlinks
* `sha256_no_links_object_format_is_propagated`
* `file://` URL + `GIT_CLONE_LOCAL_NO_LINKS` -> local code path with copy
The fallback definition of O_FSYNC uses `(1 << 31)`, which is undefined
behavior in C. The literal `1` is a signed int, and left-shifting into
the sign bit of a signed integer is undefined per the C standard.
This causes crashes on arm64 Linux with musl libc (which doesn't define
O_FSYNC), manifesting as:
thread panic: left shift of 1 by 31 places cannot be represented
in type 'int'
Fall back to O_SYNC when available, since it is the POSIX standard name
for the same flag. On platforms where neither is defined (e.g. Windows),
use (1 << 30) as a sentinel value that avoids the sign bit.
We need this in order for `pkg-config` to be able to tell what you should link
against when building libgit2 statically. We do include libssh2 in
`Libs.private` but not in `Requires.private`. The difference is a bit subtle but
has become important.
You can call `pkg-config --libs --static ${build}/libgit2.pc` and it will give
you what is in the Libs line, and also what the packages from the Requires field
have in theirs. This is what e.g. `rugged` does and it has been working until
recently. An update to openssl to require zstd now means that using `--libs
--static` returns `-lzstd` as well as many others. This means that those who
want to link using that command now need to have the development packages for
zstd installed, which should not be necessary as libgit2 itself doesn't want to
use anything from it.
A better command to use here seems to be `pkg-config --libs --static --pure
${build}/libgit2.pc`. The manpage and help output are not very precise but what
this does is limit the list of dependencies to a single layer, which is what we
want as we would only want to link statically against libgit2 and not the rest
of the libraries.
But trying to do so breaks building with libssh2 as it's included in the Libs
field rather than the Requires, so that command excludes any linking to libssh2.
Put libssh2 in the `Requires.private` field so we correctly express we need to
link to it when linking statically.
This is unfortunately an imperfect fix as now, if we did not find libssh2 via
pkg-config but rather via CMake's `find_package`, the combination of `--static
--pure` does not take `Libs.private` into account. However we only support this
as an edge case and we expect `pkg-config` to be available for the rest of our
dependencies.
