Skip to content

G
git2
Switch branch/tag
  1. 01 Oct, 2018 8 commits
  3. 06 Aug, 2018 5 commits
    • Merge pull request #4756 from pks-t/pks/v0.27.4 · 8b89f362 
      Release v0.27.4
      Patrick Steinhardt committed
    • version: bump to v0.27.4 · c5dd0ea1
      Patrick Steinhardt committed
    • CHANGELOG.md: document security release v0.27.4 · be0edb43
      Patrick Steinhardt committed
    • smart_pkt: fix potential OOB-read when processing ng packet · 1f9a8510 
      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: force usage of Xcode 8.3 image · faa7650b 
      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
  5. 09 Jul, 2018 1 commit
  7. 05 Jul, 2018 5 commits
    • version: bump to v0.27.3 · 8fbd7563
      Patrick Steinhardt committed
    • CHANGELOG: add release notes for v0.27.3 · 36f07807
      Patrick Steinhardt committed
    • delta: fix overflow when computing limit · c1577110 
      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
    • delta: fix out-of-bounds read of delta · 9844d38b 
      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
    • delta: fix sign-extension of big left-shift · 3f461902 
      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
  9. 10 Jun, 2018 1 commit
  11. 06 Jun, 2018 4 commits
    • version: bump soversion to v0.27.2 · 853ef86a
      Patrick Steinhardt committed
    • CHANGELOG.md: update for release v0.27.2 · 0818adec
      Patrick Steinhardt committed
    • tests: submodule: do not rely on config iteration order · 35865117 
      The test submodule::lookup::duplicated_path, which tries to verify that
we detect submodules with duplicated paths, currently relies on the
gitmodules file of "submod2_target". While this file has two gitmodules
with the same path, one of these gitmodules has an empty name and thus
does not pass `git_submodule_name_is_valid`. Because of this, the test
is in fact dependent on the iteration order in which we process the
submodules. In fact the "valid" submodule comes first, the "invalid"
submodule will cause the desired error. In fact the "invalid" submodule
comes first, it will be skipped due to its name being invalid, and we
will not see the desired error. While this works on the master branch
just right due to the refactoring of our config code, where iteration
order is now deterministic, this breaks on all older maintenance
branches.

Fix the issue by simply using `cl_git_rewritefile` to rewrite the
gitmodules file. This greatly simplifies the test and also makes the
intentions of it much clearer.
      Patrick Steinhardt committed
    • submodule: detect duplicated submodule paths · 7392799d 
      When loading submodule names, we build a map of submodule paths and
their respective names. While looping over the configuration keys,
we do not check though whether a submodule path was seen already. This
leads to a memory leak in case we have multiple submodules with the same
path, as we just overwrite the old value in the map in that case.

Fix the error by verifying that the path to be added is not yet part of
the string map. Git does not allow to have multiple submodules for a
path anyway, so we now do the same and detect this duplication,
reporting it to the user.
      Patrick Steinhardt committed
  13. 30 May, 2018 16 commits