- 01 Oct, 2018 1 commit
-
-
(cherry picked from commit a03113e8)
Nelson Elhage committed
-
- 06 Aug, 2018 5 commits
-
-
Release v0.26.6
Patrick Steinhardt committed -
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
OSS-fuzz has reported a potential out-of-bounds read when processing a "ng" smart packet: ==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6310000249c0 at pc 0x000000493a92 bp 0x7ffddc882cd0 sp 0x7ffddc882480 READ of size 65529 at 0x6310000249c0 thread T0 SCARINESS: 26 (multi-byte-read-heap-buffer-overflow) #0 0x493a91 in __interceptor_strchr.part.35 /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:673 #1 0x813960 in ng_pkt libgit2/src/transports/smart_pkt.c:320:14 #2 0x810f79 in git_pkt_parse_line libgit2/src/transports/smart_pkt.c:478:9 #3 0x82c3c9 in git_smart__store_refs libgit2/src/transports/smart_protocol.c:47:12 #4 0x6373a2 in git_smart__connect libgit2/src/transports/smart.c:251:15 #5 0x57688f in git_remote_connect libgit2/src/remote.c:708:15 #6 0x52e59b in LLVMFuzzerTestOneInput /src/download_refs_fuzzer.cc:145:9 #7 0x52ef3f in ExecuteFilesOnyByOne(int, char**) /src/libfuzzer/afl/afl_driver.cpp:301:5 #8 0x52f4ee in main /src/libfuzzer/afl/afl_driver.cpp:339:12 #9 0x7f6c910db82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291 #10 0x41d518 in _start When parsing an "ng" packet, we keep track of both the current position as well as the remaining length of the packet itself. But instead of taking care not to exceed the length, we pass the current pointer's position to `strchr`, which will search for a certain character until hitting NUL. It is thus possible to create a crafted packet which doesn't contain a NUL byte to trigger an out-of-bounds read. Fix the issue by instead using `memchr`, passing the remaining length as restriction. Furthermore, verify that we actually have enough bytes left to produce a match at all. OSS-Fuzz-Issue: 9406
Patrick Steinhardt committed -
Travis has upgraded the default Xcode images from 8.3 to 9.4 on 31st July 2018, including an upgrade to macOS 10.13. Unfortunately, this breaks our CI builds on our maintenance branches. As we do not want to include mayor changes to fix the integration right now, we force use of the old Xcode 8.3 images.
Patrick Steinhardt committed
-
- 09 Jul, 2018 1 commit
-
-
Release v0.26.5
Patrick Steinhardt committed
-
- 05 Jul, 2018 5 commits
-
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
When checking whether a delta base offset and length fit into the base we have in memory already, we can trigger an overflow which breaks the check. This would subsequently result in us reading memory from out of bounds of the base. The issue is easily fixed by checking for overflow when adding `off` and `len`, thus guaranteeting that we are never indexing beyond `base_len`. This corresponds to the git patch 8960844a7 (check patch_delta bounds more carefully, 2006-04-07), which adds these overflow checks. Reported-by: Riccardo Schirone <rschiron@redhat.com>
Patrick Steinhardt committed -
When computing the offset and length of the delta base, we repeatedly increment the `delta` pointer without checking whether we have advanced past its end already, which can thus result in an out-of-bounds read. Fix this by repeatedly checking whether we have reached the end. Add a test which would cause Valgrind to produce an error. Reported-by: Riccardo Schirone <rschiron@redhat.com> Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
Patrick Steinhardt committed -
Our delta code was originally adapted from JGit, which itself adapted it from git itself. Due to this heritage, we inherited a bug from git.git in how we compute the delta offset, which was fixed upstream in 48fb7deb5 (Fix big left-shifts of unsigned char, 2009-06-17). As explained by Linus: Shifting 'unsigned char' or 'unsigned short' left can result in sign extension errors, since the C integer promotion rules means that the unsigned char/short will get implicitly promoted to a signed 'int' due to the shift (or due to other operations). This normally doesn't matter, but if you shift things up sufficiently, it will now set the sign bit in 'int', and a subsequent cast to a bigger type (eg 'long' or 'unsigned long') will now sign-extend the value despite the original expression being unsigned. One example of this would be something like unsigned long size; unsigned char c; size += c << 24; where despite all the variables being unsigned, 'c << 24' ends up being a signed entity, and will get sign-extended when then doing the addition in an 'unsigned long' type. Since git uses 'unsigned char' pointers extensively, we actually have this bug in a couple of places. In our delta code, we inherited such a bogus shift when computing the offset at which the delta base is to be found. Due to the sign extension we can end up with an offset where all the bits are set. This can allow an arbitrary memory read, as the addition in `base_len < off + len` can now overflow if `off` has all its bits set. Fix the issue by casting the result of `*delta++ << 24UL` to an unsigned integer again. Add a test with a crafted delta that would actually succeed with an out-of-bounds read in case where the cast wouldn't exist. Reported-by: Riccardo Schirone <rschiron@redhat.com> Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
Patrick Steinhardt committed
-
- 04 Jun, 2018 1 commit
-
-
Release v0.26.4
Edward Thomson committed
-
- 01 Jun, 2018 23 commits
-
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Carlos Martín Nieto committed
-
Carlos Martín Nieto committed
-
Carlos Martín Nieto committed
-
We might modify caches due to us trying to load the configuration to figure out what kinds of filesystem protections we should have.
Carlos Martín Nieto committed -
We still compare case-insensitively to protect more thoroughly as we don't know what specifics we'll see on the system and it's the behaviour from git.
Carlos Martín Nieto committed -
When dealing with `core.proectNTFS` and `core.protectHFS` we do check against `.gitmodules` but we still have a failing test as the non-filesystem codepath does not check for it.
Carlos Martín Nieto committed -
Any part of the library which asks the question can pass in the mode to have it checked against `.gitmodules` being a symlink. This is particularly relevant for adding entries to the index from the worktree and for checking out files.
Carlos Martín Nieto committed -
This is so we have it available for the path validity checking. In a later commit we will start rejecting `.gitmodules` files as symlinks.
Carlos Martín Nieto committed -
We may take in names from the middle of a string so we want the caller to let us know how long the path component is that we should be checking.
Carlos Martín Nieto committed -
We want to reject these as they cause compatibility issues and can lead to git writing to files outside of the repository.
Carlos Martín Nieto committed -
These will be used by the checkout code to detect them for the particular filesystem they're on.
Carlos Martín Nieto committed -
These can't go into the public API yet as we don't want to introduce API or ABI changes in a security release.
Carlos Martín Nieto committed -
Carlos Martín Nieto committed
-
Given a path component it knows what to pass to the filesystem-specific functions so we're protected even from trees which try to use the 8.3 naming rules to get around us matching on the filename exactly. The logic and test strings come from the equivalent git change.
Carlos Martín Nieto committed -
It checks against the 8.3 shortname variants, including the one which includes the checksum as part of its name.
Carlos Martín Nieto committed -
This lets us check for other kinds of reserved files.
Carlos Martín Nieto committed -
Otherwise we would also admit `..\..\foo\bar` as a valid path and fail to protect Windows users. Ideally we would check for both separators without the need for the copied string, but this'll get us over the RCE.
Carlos Martín Nieto committed -
If the we decide that the "name" of the submodule (i.e. its path inside `.git/modules/`) is trying to escape that directory or otherwise trick us, we ignore the configuration for that submodule. This leaves us with a half-configured submodule when looking it up by path, but it's the same result as if the configuration really were missing. The name check is potentially more strict than it needs to be, but it lets us re-use the check we're doing for the checkout. The function that encapsulates this logic is ready to be exported but we don't want to do that in a security release so it remains internal for now.
Carlos Martín Nieto committed -
We should pretend such submdules do not exist as it can lead to RCE.
Carlos Martín Nieto committed -
Update the settings to use a specific read-only token for accessing our test repositories in Bitbucket.
Edward Thomson committed -
At present, we have three online tests against bitbucket: one which specifies the credentials in the payload, one which specifies the correct credentials in the URL and a final one that specifies the incorrect credentials in the URL. Bitbucket has begun responding to the latter test with a 403, which causes us to fail. Break these three tests into separate tests so that we can skip the latter until this is resolved on Bitbucket's end or until we can change the test to a different provider.
Edward Thomson committed
-
- 12 Mar, 2018 3 commits
-
-
v0.26.3 backports
Patrick Steinhardt committed -
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
- 10 Mar, 2018 1 commit
-
-
Our curl-based streams make use of the easy curl interface. This interface automatically initializes and de-initializes the global curl state by calling out to `curl_global_init` and `curl_global_cleanup`. Thus, all global state will be repeatedly re-initialized when creating multiple curl streams in succession. Despite being inefficient, this is not thread-safe due to `curl_global_init` being not thread-safe itself. Thus a multi-threaded programing handling multiple curl streams at the same time is inherently racy. Fix the issue by globally initializing and cleaning up curl's state.
Patrick Steinhardt committed
-