1. 12 Oct, 2018 2 commits
    • buf tests: allocate a smaller size for the oom · f2087fc7
      On Linux (where we run valgrind) allocate a smaller buffer, but still an
      insanely large size.  This will cause malloc to fail but will not cause
      valgrind to report a likely error with a negative-sized malloc.
      
      Keep the original buffer size on non-Linux platforms: this is
      well-tested on them and changing it may be problematic.  On macOS, for
      example, using the new size causes `malloc` to print a warning to
      stderr.
      
      (cherry picked from commit 219512e7)
      Edward Thomson committed
    • tests: simplify cmake test configuration · 373bf31f
      Simplify the names for the tests, removing the unnecessary
      "libgit2-clar" prefix.  Make "all" the new default test run, and include
      the online tests by default (since HTTPS should always be enabled).
      
      For the CI tests, create an offline-only test, then the various online
      tests.
      
      (cherry picked from commit ce798b25)
      Edward Thomson committed
  2. 05 Oct, 2018 3 commits
  3. 03 Oct, 2018 4 commits
    • smart_pkt: reorder and rename parameters of `git_pkt_parse_line` · 7e3cd611
      The parameters of the `git_pkt_parse_line` function are quite confusing.
      First, there is no real indicator what the `out` parameter is actually
      all about, and it's not really clear what the `bufflen` parameter refers
      to. Reorder and rename the parameters to make this more obvious.
      
      (cherry picked from commit 0b3dfbf4)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ok" packets · 319f0c03
      There are two different buffer overflows present when parsing "ok"
      packets. First, we never verify whether the line already ends after
      "ok", but directly go ahead and also try to skip the expected space
      after "ok". Second, we then go ahead and use `strchr` to scan for the
      terminating newline character. But in case where the line isn't
      terminated correctly, this can overflow the line buffer.
      
      Fix the issues by using `git__prefixncmp` to check for the "ok " prefix
      and only checking for a trailing '\n' instead of using `memchr`. This
      also fixes the issue of us always requiring a trailing '\n'.
      
      Reported by oss-fuzz, issue 9749:
      
      Crash Type: Heap-buffer-overflow READ {*}
      Crash Address: 0x6310000389c0
      Crash State:
        ok_pkt
        git_pkt_parse_line
        git_smart__store_refs
      
      Sanitizer: address (ASAN)
      (cherry picked from commit a9f1ca09)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ACK" packets · 0599c267
      We are being quite lenient when parsing "ACK" packets. First, we didn't
      correctly verify that we're not overrunning the provided buffer length,
      which we fix here by using `git__prefixncmp` instead of
      `git__prefixcmp`. Second, we do not verify that the actual contents make
      any sense at all, as we simply ignore errors when parsing the ACKs OID
      and any unknown status strings. This may result in a parsed packet
      structure with invalid contents, which is being silently passed to the
      caller. This is being fixed by performing proper input validation and
      checking of return codes.
      
      (cherry picked from commit bc349045)
      Patrick Steinhardt committed
    • tests: verify parsing logic for smart packets · bd069448
      The commits following this commit are about to introduce quite a lot of
      refactoring and tightening of the smart packet parser. Unfortunately, we
      do not yet have any tests despite our online tests that verify that our
      parser does not regress upon changes. This is doubly unfortunate as our
      online tests aren't executed by default.
      
      Add new tests that exercise the smart parsing logic directly by
      executing `git_pkt_parse_line`.
      
      (cherry picked from commit 365d2720)
      Patrick Steinhardt committed
  4. 05 Jul, 2018 2 commits
    • 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
  5. 06 Jun, 2018 2 commits
    • 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
  6. 30 May, 2018 11 commits
  7. 29 May, 2018 11 commits
  8. 23 Mar, 2018 2 commits
    • odb: fix writing to fake write streams · a52b4c51
      In commit 7ec7aa4a (odb: assert on logic errors when writing objects,
      2018-02-01), the check for whether we are trying to overflowing the fake
      stream buffer was changed from returning an error to raising an assert.
      The conversion forgot though that the logic around `assert`s are
      basically inverted. Previously, if the statement
      
          stream->written + len > steram->size
      
      evaluated to true, we would return a `-1`. Now we are asserting that
      this statement is true, and in case it is not we will raise an error. So
      the conversion to the `assert` in fact changed the behaviour to the
      complete opposite intention.
      
      Fix the assert by inverting its condition again and add a regression
      test.
      Patrick Steinhardt committed
    • tests: add tests for the mempack ODB backend · 904307af
      Our mempack ODB backend has no test coverage at all right now. Add a
      simple test suite to at least have some coverage of the most basic
      operations on the ODB.
      Patrick Steinhardt committed
  9. 20 Mar, 2018 1 commit
  10. 19 Mar, 2018 2 commits