1. 01 Oct, 2018 5 commits
    • Fix 'invalid packet line' for ng packets containing errors · 8ced4627
      (cherry picked from commit 50dd7fea)
      Christian Schlack committed
    • Prevent heap-buffer-overflow · ffc20564
      When running repack while doing repo writes, `packfile_load__cb()` can see some temporary files in the directory that are bigger than the usual, and makes `memcmp` overflow on the `p->pack_name` string. ASAN detected this. This just uses `strncmp`, that should not have any performance impact and is safe for comparing strings of different sizes.
      
      ```
      ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200001a3f3 at pc 0x7f4a9e1976ec bp 0x7ffc1f80e100 sp 0x7ffc1f80d8b0
      READ of size 89 at 0x61200001a3f3 thread T0
      SCARINESS: 26 (multi-byte-read-heap-buffer-overflow)
          #0 0x7f4a9e1976eb in __interceptor_memcmp.part.78 (/build/cfgr-admin#link-tree/libtools_build_sanitizers_asan-ubsan-py.so+0xcf6eb)
          #1 0x7f4a518c5431 in packfile_load__cb /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:213
          #2 0x7f4a518d9582 in git_path_direach /build/libgit2/0.27.0/src/libgit2-0.27.0/src/path.c:1134
          #3 0x7f4a518c58ad in pack_backend__refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:347
          #4 0x7f4a518c1b12 in git_odb_refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1511
          #5 0x7f4a518bff5f in git_odb__freshen /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:752
          #6 0x7f4a518c17d4 in git_odb_stream_finalize_write /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1415
          #7 0x7f4a51b9d015 in Repository_write /build/pygit2/0.27.0/src/pygit2-0.27.0/src/repository.c:509
      ```
      
      (cherry picked from commit d22cd1f4)
      bisho committed
    • config_parse: refactor error handling when parsing multiline variables · c4db1715
      The current error handling for the multiline variable parser is a bit
      fragile, as each error condition has its own code to clear memory.
      Instead, unify error handling as far as possible to avoid this
      repetitive code. While at it, make use of `GITERR_CHECK_ALLOC` to
      correctly handle OOM situations and verify that the buffer we print into
      does not run out of memory either.
      
      (cherry picked from commit bc63e1ef)
      Patrick Steinhardt committed
    • config: Fix a leak parsing multi-line config entries · c18c913c
      (cherry picked from commit 38b85255)
      Nelson Elhage committed
    • config: convert unbounded recursion into a loop · da70156e
      (cherry picked from commit a03113e8)
      Nelson Elhage committed
  2. 06 Aug, 2018 5 commits
    • Merge pull request #4757 from pks-t/pks/v0.26.6 · e98d0a37
      Release v0.26.6
      Patrick Steinhardt committed
    • smart_pkt: fix potential OOB-read when processing ng packet · 50705a2a
      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 · 7e12ca68
      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
  3. 09 Jul, 2018 1 commit
  4. 05 Jul, 2018 5 commits
    • delta: fix overflow when computing limit · 47ea1f58
      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 · 25d4a8c9
      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 · 8ab4f363
      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
  5. 04 Jun, 2018 1 commit
  6. 01 Jun, 2018 23 commits