1. 26 Mar, 2020 11 commits
    • config: check if we are running in a sandboxed environment · 30cd1e1f
      On macOS the $HOME environment variable returns the path to the sandbox container instead of the actual user $HOME for sandboxed apps. To get the correct path, we have to get it from the password file entry.
      Erik Aigner committed
    • patch_parse: fix segfault due to line containing static contents · c159cceb
      With commit dedf70ad (patch_parse: do not depend on parsed buffer's
      lifetime, 2019-07-05), all lines of the patch are allocated with
      `strdup` to make lifetime of the parsed patch independent of the buffer
      that is currently being parsed. In patch b0893282 (patch_parse: ensure
      valid patch output with EOFNL, 2019-07-11), we introduced another
      code location where we add lines to the parsed patch. But as that one
      was implemented via a separate pull request, it wasn't converted to use
      `strdup`, as well. As a consequence, we generate a segfault when trying
      to deallocate the potentially static buffer that's now in some of the
      lines.
      
      Use `git__strdup` to fix the issue.
      Patrick Steinhardt committed
    • patch_parse: handle missing newline indicator in old file · fe012c60
      When either the old or new file contents have no newline at the end of
      the file, then git-diff(1) will print out a "\ No newline at end of
      file" indicator. While we do correctly handle this in the case where the
      new file has this indcator, we fail to parse patches where the old file
      is missing a newline at EOF.
      
      Fix this bug by handling and missing newline indicators in the old file.
      Add tests to verify that we can parse such files.
      Patrick Steinhardt committed
    • 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 3 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