1. 29 Jun, 2018 2 commits
    • delta: fix out-of-bounds read of delta · 24597812
      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 · 7db25870
      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
  2. 27 Jun, 2018 3 commits
  3. 26 Jun, 2018 2 commits
  4. 25 Jun, 2018 7 commits
  5. 24 Jun, 2018 3 commits
  6. 22 Jun, 2018 6 commits
    • Clear revwalk sorting when resetting · 4fd81c53
      Currently we fail to clear the sorting flag for revwalks when resetting.
      This caused a poor interaction with the limited flag during a recent
      patch. This patch clears the revwalk sorting flag and causes it to no
      longer persist over resets.
      Nika Layzell committed
    • deps: fix implicit fallthrough warning in http-parser · cacbf998
      GCC 7 has introduced new warnings for implicit fallthrough in switch
      statements. Whenever there is no branch in a case block, GCC will watch
      out for some heuristics which indicate that the implicit fallthrough is
      intended, like a "fallthrough" comment. The third-party http-parser code
      manages to trick this heuristic in one case, even though there is a
      "FALLTHROUGH" comment. Fortunately, GCC has also added a strictness
      level to the -Wimplicit-fallthrough diagnostic, such that we can loosen
      this heuristic and make it more lax.
      
      Set -Wimplicit-fallthrough=1 in http-parser's CMake build instructions,
      which is the strictest level that gets rid of the warning. This level
      will treat any kind of comment as a "fallthrough" comment, which
      silences the warning.
      Patrick Steinhardt committed
    • Merge pull request #4411 from pks-t/pks/config-parse-cleanups · b121b7ac
      Config parser cleanups
      Edward Thomson committed
    • config_file: avoid free'ing OOM buffers · e1e90dcc
      Buffers which ran out of memory will never have any memory attached to
      them. As such, it is not necessary to call `git_buf_free` if the buffer
      is out of memory.
      Patrick Steinhardt committed
    • config_parse: always sanitize out-parameters in `parse_variable` · 83b5f161
      The `parse_variable` function has two out parameters `var_name` and
      `var_value`. Currently, those are not being sanitized to `NULL`. when.
      any error happens inside of the `parse_variable` function. Fix that.
      While at it, the coding style is improved to match our usual coding
      practices more closely.
      Patrick Steinhardt committed
    • config_parse: have `git_config_parse` own entry value and name · e51e29e8
      The function `git_config_parse` uses several callbacks to pass data
      along to the caller as it parses the file. One design shortcoming here
      is that strings passed to those callbacks are expected to be freed by
      them, which is really confusing.
      
      Fix the issue by changing memory ownership here. Instead of expecting
      the `on_variable` callbacks to free memory for `git_config_parse`, just
      do it inside of `git_config_parse`. While this obviously requires a bit
      more memory allocation churn due to having to copy both name and value
      at some places, this shouldn't be too much of a burden.
      Patrick Steinhardt committed
  7. 18 Jun, 2018 5 commits
  8. 17 Jun, 2018 1 commit
  9. 16 Jun, 2018 1 commit
  10. 15 Jun, 2018 10 commits