1. 26 Oct, 2018 2 commits
    • buf tests: allocate a smaller size for the oom · 69e86433
      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
    • clar: verify command line arguments before execute · e5278f70
      When executing `libgit2_clar -smerge -invalid_option`, it will first execute
      the merge test suite and afterwards output help because of the invalid option.
      
      With this changa, it verifies all options before execute. If there are any
      invalid options, it will output help and exit without actually executing
      the test suites.
      
      (cherry picked from commit 32758631)
      Yoney committed
  2. 19 Oct, 2018 7 commits
    • tests: online::clone: inline creds-test with nonexistent URL · 2362ce6c
      Right now, we test our credential callback code twice, once via SSH on
      localhost and once via a non-existent GitHub repository. While the first
      URL makes sense to be configurable, it does not make sense to hard-code
      the non-existing repository, which requires us to call tests multiple
      times. Instead, we can just inline the URL into another set of tests.
      
      (cherry picked from commit 54a1bf05)
      Patrick Steinhardt committed
    • tests: online::clone: construct credential-URL from environment · a1a495f2
      We support two types of passing credentials to the proxy, either via the
      URL or explicitly by specifying user and password. We test these types
      by modifying the proxy URL and executing the tests twice, which is
      in fact unnecessary and requires us to maintain the list of environment
      variables and test executions across multiple CI infrastructures.
      
      To fix the situation, we can just always pass the host, port, user and
      password to the tests. The tests can then assemble the complete URL
      either with or without included credentials, allowing us to test both
      cases in-process.
      
      (cherry picked from commit fea60920)
      Patrick Steinhardt committed
    • tests: perf: build but exclude performance tests by default · 89641431
      Our performance tests (or to be more concrete, our single performance
      test) are not built by default, as they are always #ifdef'd out. While
      it is true that we don't want to run performance tests by default, not
      compiling them at all may cause code rot and is thus an unfavorable
      approach to handle this.
      
      We can easily improve this situation: this commit removes the #ifdef,
      causing the code to always be compiled. Furthermore, we add `-xperf` to
      the default command line parameters of `generate.py`, thus causing the
      tests to be excluded by default.
      
      Due to this approach, we are now able to execute the performance tests
      by passing `-sperf` to `libgit2_clar`. Unfortunately, we cannot execute
      the performance tests on Travis or AppVeyor as they rely on history
      being available for the libgit2 repository. As both do a shallow clone
      only, though, this is not given.
      
      (cherry picked from commit 543ec149)
      Patrick Steinhardt committed
    • tests: iterator::workdir: fix reference count in stale test · 98378a3f
      The test `iterator::workdir::filesystem_gunk` is usually not executed,
      as it is guarded by the environment variable "GITTEST_INVASIVE_SPEED"
      due to its effects on speed. As such, it has become stale and does not
      account for new references which have meanwhile been added to the
      testrepo, causing it to fail. Fix this by raising the number of expected
      references to 15.
      
      (cherry picked from commit b8c14499)
      Patrick Steinhardt committed
    • tests: iterator_helpers: assert number of iterator items · d2bbea82
      When the function `expect_iterator_items` surpasses the number of
      expected items, we simply break the loop. This causes us to trigger an
      assert later on which has message attached, which is annoying when
      trying to locate the root error cause. Instead, directly assert that the
      current count is still smaller or equal to the expected count inside of
      the loop.
      
      (cherry picked from commit 9aba7636)
      Patrick Steinhardt committed
    • tests: status::worktree: indicate skipped tests on Win32 · 293c5ef2
      Some function bodies of tests which are not applicable to the Win32
      platform are completely #ifdef'd out instead of calling `cl_skip()`.
      This leaves us with no indication that these tests are not being
      executed at all and may thus cause decreased scrutiny when investigating
      skipped tests. Improve the situation by calling `cl_skip()` instead of
      just doing nothing.
      
      (cherry picked from commit 72c28ab0)
      Patrick Steinhardt committed
    • tests: online::clone: use URL of test server · b988f544
      All our tests running against a local SSH server usually read the
      server's URL from environment variables. But online::clone::ssh_cert
      test fails to do so and instead always connects to
      "ssh://localhost/foo". This assumption breaks whenever the SSH server is
      not running on the standard port, e.g. when it is running as a user.
      
      Fix the issue by using the URL provided by the environment.
      
      (cherry picked from commit c2c95ad0)
      Patrick Steinhardt committed
  3. 05 Oct, 2018 3 commits
  4. 03 Oct, 2018 4 commits
    • smart_pkt: reorder and rename parameters of `git_pkt_parse_line` · 3bbda7d7
      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 · 8cd0a897
      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 · 82c3fc33
      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 · 5d108c9a
      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
  5. 01 Oct, 2018 1 commit
    • util: introduce `git__prefixncmp` and consolidate implementations · a8db6c92
      Introduce `git_prefixncmp` that will search up to the first `n`
      characters of a string to see if it is prefixed by another string.
      This is useful for examining if a non-null terminated character
      array is prefixed by a particular substring.
      
      Consolidate the various implementations of `git__prefixcmp` around a
      single core implementation and add some test cases to validate its
      behavior.
      
      (cherry picked from commit 86219f40)
      Edward Thomson committed
  6. 05 Jul, 2018 2 commits
    • 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
  7. 01 Jun, 2018 13 commits
  8. 10 Mar, 2018 8 commits