1. 10 Dec, 2019 11 commits
    • path: protect NTFS everywhere · 50a33c30
      Enable core.protectNTFS by default everywhere and in every codepath, not
      just on checkout.
      Edward Thomson committed
    • test: ensure we can't add a protected path · aa0902f4
      Test that when we enable core.protectNTFS that we cannot add
      platform-specific invalid paths to the index.
      Edward Thomson committed
    • test: improve badname verification test · f26b03d9
      The name of the `add_invalid_filename` function suggests that we
      _want_ to add an invalid filename.  Rename the function to show that
      we expect to _fail_ to add the invalid filename.
      Edward Thomson committed
    • test: ensure treebuilder validate new protection rules · 94589e7c
      Ensure that the new protection around .git::$INDEX_ALLOCATION rules are
      enabled for using the treebuilder when core.protectNTFS is set.
      Edward Thomson committed
    • test: ensure index adds validate new protection rules · fd255d2c
      Ensure that the new protection around .git::$INDEX_ALLOCATION rules are
      enabled for adding to the index when core.protectNTFS is set.
      Edward Thomson committed
    • test: improve badname verification test · a336ed18
      The name of the `write_invalid_filename` function suggests that we
      _want_ to write an invalid filename.  Rename the function to show that
      we expect to _fail_ to write the invalid filename.
      Edward Thomson committed
    • path: rename function that detects end of filename · f49378b6
      The function `only_spaces_and_dots` used to detect the end of the
      filename on win32.  Now we look at spaces and dots _before_ the end of
      the string _or_ a `:` character, which would signify a win32 alternate
      data stream.
      
      Thus, rename the function `ntfs_end_of_filename` to indicate that it
      detects the (virtual) end of a filename, that any further characters
      would be elided to the given path.
      Edward Thomson committed
    • path: also guard `.gitmodules` against NTFS Alternate Data Streams · ac0b2ef1
      We just safe-guarded `.git` against NTFS Alternate Data Stream-related
      attack vectors, and now it is time to do the same for `.gitmodules`.
      
      Note: In the added regression test, we refrain from verifying all kinds
      of variations between short names and NTFS Alternate Data Streams: as
      the new code disallows _all_ Alternate Data Streams of `.gitmodules`, it
      is enough to test one in order to know that all of them are guarded
      against.
      
      Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      Johannes Schindelin committed
    • Disallow NTFS Alternate Data Stream attacks, even on Linux/macOS · 460a9fdc
      A little-known feature of NTFS is that it offers to store metadata in
      so-called "Alternate Data Streams" (inspired by Apple's "resource
      forks") that are copied together with the file they are associated with.
      These Alternate Data Streams can be accessed via `<file name>:<stream
      name>:<stream type>`.
      
      Directories, too, have Alternate Data Streams, and they even have a
      default stream type `$INDEX_ALLOCATION`. Which means that `abc/` and
      `abc::$INDEX_ALLOCATION/` are actually equivalent.
      
      This is of course another attack vector on the Git directory that we
      definitely want to prevent.
      
      On Windows, we already do this incidentally, by disallowing colons in
      file/directory names.
      
      While it looks as if files'/directories' Alternate Data Streams are not
      accessible in the Windows Subsystem for Linux, and neither via
      CIFS/SMB-mounted network shares in Linux, it _is_ possible to access
      them on SMB-mounted network shares on macOS.
      
      Therefore, let's go the extra mile and prevent this particular attack
      _everywhere_. To keep things simple, let's just disallow *any* Alternate
      Data Stream of `.git`.
      
      This is libgit2's variant of CVE-2019-1352.
      
      Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      Johannes Schindelin committed
    • Protect against 8.3 "short name" attacks also on Linux/macOS · 7bf80ab0
      The Windows Subsystem for Linux (WSL) is getting increasingly popular,
      in particular because it makes it _so_ easy to run Linux software on
      Windows' files, via the auto-mounted Windows drives (`C:\` is mapped to
      `/mnt/c/`, no need to set that up manually).
      
      Unfortunately, files/directories on the Windows drives can be accessed
      via their _short names_, if that feature is enabled (which it is on the
      `C:` drive by default).
      
      Which means that we have to safeguard even our Linux users against the
      short name attacks.
      
      Further, while the default options of CIFS/SMB-mounts seem to disallow
      accessing files on network shares via their short names on Linux/macOS,
      it _is_ possible to do so with the right options.
      
      So let's just safe-guard against short name attacks _everywhere_.
      
      Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      Johannes Schindelin committed
    • cl_git_fail: do not report bogus error message · 48043516
      When we expect a checkout operation to fail, but it succeeds, we
      actually do not want to see the error messages that were generated in
      the meantime for errors that were handled gracefully by the code (e.g.
      when an object could not be found in a pack: in this case, the next
      backend would have been given a chance to look up the object, and
      probably would have found it because the checkout succeeded, after all).
      
      Which means that in the specific case of `cl_git_fail()`, we actually
      want to clear the global error state _after_ evaluating the command: we
      know that any still-available error would be bogus, seeing as the
      command succeeded (unexpectedly).
      
      Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      Johannes Schindelin committed
  2. 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
  3. 21 May, 2019 1 commit
  4. 20 May, 2019 4 commits
  5. 02 May, 2019 16 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
    • patch_parse: fix parsing addition/deletion of file with space · 96b6fe11
      The diff header format is a strange beast in that it is inherently
      unparseable in an unambiguous way. While parsing
      
          a/file.txt b/file.txt
      
      is obvious and trivially doable, parsing a diff header of
      
          a/file b/file ab.txt b/file b/file ab.txt
      
      is not (but in fact valid and created by git.git).
      
      Due to that, we have relaxed our diff header parser in commit 80226b5f
      (patch_parse: allow parsing ambiguous patch headers, 2017-09-22), so
      that we started to bail out when seeing diff headers with spaces in
      their file names. Instead, we try to use the "---" and "+++" lines,
      which are unambiguous.
      
      In some cases, though, we neither have a useable file name from the
      header nor from the "---" or "+++" lines. This is the case when we have
      a deletion or addition of a file with spaces: the header is unparseable
      and the other lines will simply show "/dev/null". This trips our parsing
      logic when we try to extract the prefix (the "a/" part) that is being
      used in the path line, where we unconditionally try to dereference a
      NULL pointer in such a scenario.
      
      We can fix this by simply not trying to parse the prefix in cases where
      we have no useable path name. That'd leave the parsed patch without
      either `old_prefix` or `new_prefix` populated. But in fact such cases
      are already handled by users of the patch object, which simply opt to
      use the default prefixes in that case.
      Patrick Steinhardt committed
    • ignore: Do not match on prefix of negated patterns · 12bc7181
      Matching on the prefix of a negated pattern was triggering false
      negatives on siblings of that pattern. e.g.
      
      Given the .gitignore:
      dir/*
      !dir/sub1/sub2/**
      
      The path `dir/a.text` would not be ignored.
      Steve King Jr committed
    • Implement failing test for gitignore of complex subdirectory negation · aa877e09
      When a directory's contents are ignored, and then a glob negation is made to a nested subdir, other subdirectories are now unignored
      Tyler Ang-Wanek committed
    • Fix a _very_ improbable memory leak in git_odb_new() · 856afc27
      This change fixes a mostly theoretical memory leak in got_odb_new()
      that can only manifest if git_cache_init() fails due to running out of
      memory or not being able to acquire its lock.
      lhchavez committed
    • Fix a memory leak in odb_otype_fast() · 5189beb0
      This change frees a copy of a cached object in odb_otype_fast().
      lhchavez committed
  6. 14 Feb, 2019 4 commits