1. 26 Mar, 2020 6 commits
    • diff: ignore EOFNL for computing patch IDs · ef1651e6
      The patch ID is supposed to be mostly context-insignificant and
      thus only includes added or deleted lines. As such, we shouldn't honor
      end-of-file-without-newline markers in diffs.
      
      Ignore such lines to fix how we compute the patch ID for such diffs.
      Patrick Steinhardt committed
    • patch_parse: do not depend on parsed buffer's lifetime · 782bc334
      When parsing a patch from a buffer, we let the patch lines point into
      the original buffer. While this is efficient use of resources, this also
      ties the lifetime of the parsed patch to the parsed buffer. As this
      behaviour is not documented anywhere in our API it is very surprising to
      its users.
      
      Untie the lifetime by duplicating the lines into the parsed patch. Add a
      test that verifies that lifetimes are indeed independent of each other.
      Patrick Steinhardt committed
    • ci: add flaky test re-execution on Windows · 7786d7e9
      Our online tests are occasionally flaky since they hit real network
      endpoints.  Re-run them up to 5 times if they fail, to allow us to
      avoid having to fail the whole build.
      Edward Thomson committed
    • ci: add flaky test re-execution on Unix · f8a09985
      Our online tests are occasionally flaky since they hit real network
      endpoints.  Re-run them up to 5 times if they fail, to allow us to
      avoid having to fail the whole build.
      Edward Thomson committed
    • tests: apply: verify that we correctly truncate the source buffer · 2ce6eddf
      Previously, we would fail to correctly truncate the source buffer
      if the source has more than one line and ends with a non-newline
      character. In the following call, we thus truncate the source
      string in the middle of the second line. Without the bug fixed,
      we would successfully apply the patch to the source and return
      success. With the overflow being fixed, we should return an
      error now.
      Patrick Steinhardt committed
    • apply: prevent OOB read when parsing source buffer · 6f351d83
      When parsing the patch image from a string, we split the string
      by newlines to get a line-based view of it. To split, we use
      `memchr` on the buffer and limit the buffer length by the
      original length provided by the caller. This works just fine for
      the first line, but for every subsequent line we need to actually
      subtract the amount of bytes that we have already read.
      
      The above issue can be easily triggered by having a source buffer
      with at least two lines, where the second line does _not_ end in
      a newline. Given a string "foo\nb", we have an original length of
      five bytes. After having extracted the first line, we will point
      to 'b' and again try to `memchr(p, '\n', 5)`, resulting in an
      out-of-bounds read of four bytes.
      
      Fix the issue by correctly subtracting the amount of bytes
      already read.
      Erik Aigner committed
  2. 10 Dec, 2019 17 commits
  3. 04 Aug, 2019 4 commits
    • Release v0.28.3 · 7ce88e66
      Edward Thomson committed
    • commit_list: fix possible buffer overflow in `commit_quick_parse` · 3316f666
      The function `commit_quick_parse` provides a way to quickly parse
      parts of a commit without storing or verifying most of its
      metadata. The first thing it does is calculating the number of
      parents by skipping "parent " lines until it finds the first
      non-parent line. Afterwards, this parent count is passed to
      `alloc_parents`, which will allocate an array to store all the
      parent.
      
      To calculate the amount of storage required for the parents
      array, `alloc_parents` simply multiplicates the number of parents
      with the respective elements's size. This already screams "buffer
      overflow", and in fact this problem is getting worse by the
      result being cast to an `uint32_t`.
      
      In fact, triggering this is possible: git-hash-object(1) will
      happily write a commit with multiple millions of parents for you.
      I've stopped at 67,108,864 parents as git-hash-object(1)
      unfortunately soaks up the complete object without streaming
      anything to disk and thus will cause an OOM situation at a later
      point. The point here is: this commit was about 4.1GB of size but
      compressed down to 24MB and thus easy to distribute.
      
      The above doesn't yet trigger the buffer overflow, thus. As the
      array's elements are all pointers which are 8 bytes on 64 bit, we
      need a total of 536,870,912 parents to trigger the overflow to
      `0`. The effect is that we're now underallocating the array
      and do an out-of-bound writes. As the buffer is kindly provided
      by the adversary, this may easily result in code execution.
      
      Extrapolating from the test file with 67m commits to the one with
      536m commits results in a factor of 8. Thus the uncompressed
      contents would be about 32GB in size and the compressed ones
      192MB. While still easily distributable via the network, only
      servers will have that amount of RAM and not cause an
      out-of-memory condition previous to triggering the overflow. This
      at least makes this attack not an easy vector for client-side use
      of libgit2.
      Patrick Steinhardt committed
    • config: validate ownership of C:\ProgramData\Git\config before using it · d475d5d6
      When the VirtualStore feature is in effect, it is safe to let random
      users write into C:\ProgramData because other users won't see those
      files. This seemed to be the case when we introduced support for
      C:\ProgramData\Git\config.
      
      However, when that feature is not in effect (which seems to be the case
      in newer Windows 10 versions), we'd rather not use those files unless
      they come from a trusted source, such as an administrator.
      
      This change imitates the strategy chosen by PowerShell's native OpenSSH
      port to Windows regarding host key files: if a system file is owned
      neither by an administrator, a system account, or the current user, it
      is ignored.
      Johannes Schindelin committed
  4. 21 May, 2019 1 commit
  5. 20 May, 2019 4 commits
  6. 02 May, 2019 8 commits
    • Correctly write to missing locked global config · 98c11905
      Opening a default config when ~/.gitconfig doesn't exist, locking it,
      and attempting to write to it causes an assertion failure.
      
      Treat non-existent global config file content as an empty string.
      
      (cherry picked from commit e44110db)
      Ian Hattendorf committed
    • cmake: fix include ordering issues with bundled deps · 9e07f684
      When linking against bundled libraries, we include their header
      directories by using "-isystem". The reason for that is that we
      want to handle our vendored library headers specially, most
      importantly to ignore warnings generated by including them. By
      using "-isystem", though, we screw up the order of searched
      include directories by moving those bundled dependencies towards
      the end of the lookup order. Like this, chances are high that any
      other specified include directory contains a file that collides
      with the actual desired include file.
      
      Fix this by not treating the bundled dependencies' include
      directories as system includes. This will move them to the front
      of the lookup order and thus cause them to override
      system-provided headers. While this may cause the compiler to
      generate additional warnings when processing bundled headers,
      this is a tradeoff we should make regardless to fix builds on
      systems hitting this issue.
      
      (cherry picked from commit ee3d71fb)
      Patrick Steinhardt committed
    • cmake: correctly detect if system provides `regcomp` · 085ed2c6
      We assume that if we are on Win32, Amiga OS, Solaris or SunOS,
      that the regcomp(3P) function cannot be provided by the system.
      Thus we will in these cases always include our own, bundled regex
      sources to make a regcomp implementation available. This test is
      obviously very fragile, and we have seen it fail on MSYS2/MinGW
      systems, which do in fact provide the regcomp symbol. The effect
      is that during compilation, we will use the "regex.h" header
      provided by MinGW, but use symbols provided by ourselves. This
      in fact may cause subtle memory layout issues, as the structure
      made available via MinGW doesn't match what our bundled code
      expects.
      
      There's one more problem with our regex detection: on the listed
      platforms, we will incorrectly include the bundled regex code
      even in case where the system provides regcomp_l(3), but it will
      never be used for anything.
      
      Fix the issue by improving our regcomp detection code. Instead of
      relying on a fragile listing of platforms, we can just use
      `CHECK_FUNCTION_EXISTS` instead. This will not in fact avoid the
      header-ordering problem. But we can assume that as soon as a
      system-provided "regex.h" header is provided, that
      `CHECK_FUNCTION_EXISTS` will now correctly find the desired
      symbol and thus not include our bundled regex code.
      
      (cherry picked from commit 13cb9f7a)
      Patrick Steinhardt committed
    • config_file: check result of git_array_alloc · 4ec32d36
      git_array_alloc can return NULL if no memory is available, causing
      a segmentation fault in memset. This adds GIT_ERROR_CHECK_ALLOC
      similar to how other parts of the code base deal with the return
      value of git_array_alloc.
      Tobias Nießen committed
    • git_repository_init: stop traversing at windows root · 9b698978
      Stop traversing the filesystem at the Windows directory root.  We were
      calculating the filesystem root for the given directory to create, and
      walking up the filesystem hierarchy.  We intended to stop when the
      traversal path length is equal to the root path length (ie, stopping at
      the root, since no path may be shorter than the root path).
      
      However, on Windows, the root path may be specified in two different
      ways, as either `Z:` or `Z:\`, where `Z:` is the current drive letter.
      `git_path_dirname_r` returns the path _without_ a trailing slash, even
      for the Windows root.  As a result, during traversal, we need to test
      that the traversal path is _less than or equal to_ the root path length
      to determine if we've hit the root to ensure that we stop when our
      traversal path is `Z:` and our calculated root path was `Z:\`.
      Edward Thomson committed
    • ignore: treat paths with trailing "/" as directories · 21baf7ab
      The function `git_ignore_path_is_ignored` is there to test the
      ignore status of paths that need not necessarily exist inside of
      a repository. This has the implication that for a given path, we
      cannot always decide whether it references a directory or a file,
      and we need to distinguish those cases because ignore rules may
      treat those differently. E.g. given the following gitignore file:
      
          *
          !/**/
      
      we'd only want to unignore directories, while keeping files
      ignored. But still, calling `git_ignore_path_is_ignored("dir/")`
      will say that this directory is ignored because it treats "dir/"
      as a file path.
      
      As said, the `is_ignored` function cannot always decide whether
      the given path is a file or directory, and thus it may produce
      wrong results in some cases. While this is unfixable in the
      general case, we can do better when we are being passed a path
      name with a trailing path separator (e.g. "dir/") and always
      treat them as directories.
      Patrick Steinhardt committed
    • tests: diff: test parsing diffs with a new file with spaces in its path · 4ec209cd
      Add a test that verifies that we are able to parse patches which add a
      new file that has spaces in its path.
      Erik Aigner committed