1. 23 Aug, 2019 7 commits
    • xdiff: catch memory allocation errors · f3b3e543
      The xdiff code contains multiple call sites where the results of
      `xdl_malloc` are not being checked for memory allocation errors.
      
      Add checks to fix possible segfaults due to `NULL` pointer accesses.
      Patrick Steinhardt committed
    • transports: http: check for memory allocation failures · c2dd895a
      When allocating a chunk that is used to write to HTTP streams, we do not
      check for memory allocation errors. This may lead us to write to a
      `NULL` pointer and thus cause a segfault.
      
      Fix this by adding a call to `GIT_ERROR_CHECK_ALLOC`.
      Patrick Steinhardt committed
    • trailer: check for memory allocation errors · 08699541
      The "trailer.c" code has been copied mostly verbatim from git.git with
      minor adjustments, only. As git.git's `xmalloc` function, which aborts
      on memory allocation errors, has been swapped out for `git_malloc`,
      which doesn't abort, we may inadvertently access `NULL` pointers.
      
      Add checks to fix this.
      Patrick Steinhardt committed
    • posix: fix direct use of `malloc` · 8c7d9761
      In "posix.c" there are multiple callsites which execute `malloc` instead
      of `git__malloc`. Thus, users of library are not able to track these
      allocations with a custom allocator.
      
      Convert these call sites to use `git__malloc` instead.
      Patrick Steinhardt committed
    • indexer: catch OOM when adding expected OIDs · a477bff1
      When adding OIDs to the indexer's map of yet-to-be-seen OIDs to verify
      that packfiles are complete, we do so by first allocating a new OID and
      then calling `git_oidmap_set` on it. There was no check for memory
      allocation errors in place, though, leading to possible segfaults due to
      trying to copy data to a `NULL` pointer.
      
      Verify the result of `git__malloc` with `GIT_ERROR_CHECK_ALLOC` to fix
      the issue.
      Patrick Steinhardt committed
    • merge: check return value of `git_commit_list_insert` · d4fe402b
      The function `git_commit_list_insert` dynamically allocates memory and
      may thus fail to insert a given commit, but we didn't check for that in
      several places in "merge.c".
      
      Convert surrounding functions to return error codes and check whether
      `git_commit_list_insert` was successful, returning an error if not.
      Patrick Steinhardt committed
    • blame_git: detect memory allocation errors · c0486188
      The code in "blame_git.c" was mostly imported from git.git with only
      minor changes. One of these changes was to use our own allocators
      instead of git's `xmalloc`, but there's a subtle difference: `xmalloc`
      would abort the program if unable to allocate any memory, bit
      `git__malloc` doesn't. As we didn't check for memory allocation errors
      in some places, we might inadvertently dereference a `NULL` pointer in
      out-of-memory situations.
      
      Convert multiple functions to return proper error codes and add calls to
      `GIT_ERROR_CHECK_ALLOC` to fix this.
      Patrick Steinhardt committed
  2. 14 Aug, 2019 1 commit
  3. 13 Aug, 2019 5 commits
    • Merge pull request #5202 from libgit2/users/ethomson/security_updates · 08cfa43d
      Security updates from 0.28.3
      Edward Thomson committed
    • commit_list: fix possible buffer overflow in `commit_quick_parse` · 57a9ccd5
      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 · cb1439c9
      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
    • clone: Remove whitespace ssh test · 62b80138
      Will add later when infrastructure is configured
      Ian Hattendorf committed
  4. 12 Aug, 2019 1 commit
  5. 11 Aug, 2019 2 commits
  6. 09 Aug, 2019 1 commit
  7. 07 Aug, 2019 1 commit
  8. 02 Aug, 2019 4 commits
  9. 01 Aug, 2019 11 commits
  10. 29 Jul, 2019 2 commits
  11. 26 Jul, 2019 5 commits
    • config_backend: rename internal structures · 37ebe9ad
      The internal backend structures are kind-of legacy and do not really
      speak for themselves. Rename them accordingly to make them easier to
      understand.
      Patrick Steinhardt committed
    • config_file: separate out read-only backend · 2bff84ba
      To further distinguish the file writeable and readonly backends,
      separate the readonly backend into its own "config_snapshot.c"
      implementation. The snapshot backend can be generically used to snapshot
      any type of backend.
      Patrick Steinhardt committed
    • config_file: fix cast of readonly backend · f0b10066
      In `backend_readonly_free`, the passed in config backend is being cast
      to a `diskfile_backend` instead of to a `diskfile_readonly_backend`.
      While this works out just fine because we only access its header values,
      which were shared between both backends, it is undefined behaviour.
      
      Use the correct type to fix this.
      Patrick Steinhardt committed
    • config_file: remove shared `diskfile_header` struct · a3159df8
      The `diskfile_header` structure is shared between both
      `diskfile_backend` and `diskfile_readonly_backend`. The separation and
      resulting casting is confusing at times and a source for programming
      errors.
      
      Remove the shared structure and inline them directly.
      Patrick Steinhardt committed
    • config_file: duplicate accessors for readonly backend · 271e5fba
      While most functions of the readonly configuration backend are
      implemented separately from the writeable configuration backend, the two
      functions `config_iterator_new` and `config_get` are shared between
      both. This sharing makes it necessary to have some shared data
      structures, which is the `diskfile_header` structure. Unfortunately, this
      makes the backends harder to grasp than necessary due to all the casting
      between structs and also quite error prone.
      
      Reimplement those functions for the readonly backends. As readonly
      backends cannot be refreshed anyway, we can remove the calls to
      `config_refresh` in there.
      Patrick Steinhardt committed