1. 05 Nov, 2019 6 commits
    • config_entries: micro-optimize storage of multivars · b7dcea04
      Multivars are configuration entries that have many values for the same
      name; we can thus micro-optimize this case by just retaining the name of
      the first configuration entry and freeing all the others, letting them
      point to the string of the first entry.
      
      The attached test case is an extreme example that demonstrates this. It
      contains a section name that is approximately 500kB in size with 20.000
      entries "a=b". Without the optimization, this would require at least
      20000*500kB bytes, which is around 10GB. With this patch, it only
      requires 500kB+20000*1B=20500kB.
      
      The obvious culprit here is the section header, which we repeatedly
      include in each of the configuration entry's names. This makes it very
      easier for an adversary to provide a small configuration file that
      disproportionally blows up in memory during processing and is thus a
      feasible way for a denial-of-service attack. Unfortunately, we cannot
      fix the root cause by e.g. having a separate "section" field that may
      easily be deduplicated due to the `git_config_entry` structure being
      part of our public API. So this micro-optimization is the best we can do
      for now.
      Patrick Steinhardt committed
    • config_entries: only keep track of a single entry list · 62320860
      Whenever adding a configuration entry to the config entries structure,
      we allocate two list heads:
      
          - The first list head is added to the global list of config entries
            in order to be able to iterate over configuration entries in the
            order they were originally added.
      
          - The second list head is added to the map of entries in order to
            efficiently look up an entry by its name. If no entry with the
            same name exists in the map, then we add the new entry to the map
            directly. Otherwise, we append the new entry's list head to the
            pre-existing entry's list in order to keep track of multivars.
      
      While the former usecase is perfectly sound, the second usecase can be
      optimized. The only reason why we keep track of multivar entries in
      another separate list is to be able to determine whether an entry is
      unique or not by seeing whether its `next` pointer is set. So we keep
      track of a complete list of multivar entries just to have a single bit
      of information of whether it has other multivar entries with the same
      entry name.
      
      We can completely get rid of this secondary list by just adding a
      `first` field to the list structure itself. When executing
      `git_config_entries_append`, we will then simply check whether the
      configuration map already has an entry with the same name -- if so, we
      will set the `first` to zero to indicate that it is not the initial
      entry anymore. Instead of a second list head in the map, we can thus now
      directly store the list head of the first global list inside of the map
      and just refer to that bit.
      
      Note that the more obvious solution would be to store a `unique` field
      instead of a `first` field. But as we will only ever inspect the `first`
      field of the _last_ entry that has been moved into the map, these are
      semantically equivalent in that case.
      
      Having a `first` field also allows for a minor optimization: for
      multivar values, we can free the `name` field of all entries that are
      _not_ first and have them point to the name of the first entry instead.
      Patrick Steinhardt committed
    • config_entries: mark local functions as static · 8a88701e
      Some functions which are only used in "config_entries.c" are not marked
      as static, which is being fixed by this very commit.
      Patrick Steinhardt committed
    • Merge pull request #5293 from csware/config_snapshot-snapshot · 82d7a114
      Fix crash if snapshotting a config_snapshot
      Patrick Steinhardt committed
    • Merge pull request #5295 from romkatv/fix-diff-res · 45c8d3f4
      fix a bug introduced in 8a23597b
      Patrick Steinhardt committed
    • fix a bug introduced in 8a23597b · 1886478d
      romkatv committed
  2. 02 Nov, 2019 1 commit
  3. 01 Nov, 2019 2 commits
  4. 30 Oct, 2019 2 commits
  5. 29 Oct, 2019 1 commit
  6. 24 Oct, 2019 2 commits
  7. 22 Oct, 2019 1 commit
  8. 21 Oct, 2019 1 commit
    • patch_parse: detect overflow when calculating old/new line position · 37141ff7
      When the patch contains lines close to INT_MAX, then it may happen that
      we end up with an integer overflow when calculating the line of the
      current diff hunk. Reject such patches as unreasonable to avoid the
      integer overflow.
      
      As the calculation is performed on integers, we introduce two new
      helpers `git__add_int_overflow` and `git__sub_int_overflow` that perform
      the integer overflow check in a generic way.
      Patrick Steinhardt committed
  9. 19 Oct, 2019 3 commits
    • patch_parse: fix out-of-bounds read with No-NL lines · 468e3ddc
      We've got two locations where we copy lines into the patch. The first
      one is when copying normal " ", "-" or "+" lines, while the second
      location gets executed when we copy "\ No newline at end of file" lines.
      While the first one correctly uses `git__strndup` to copy only until the
      newline, the other one doesn't. Thus, if the line occurs at the end of
      the patch and if there is no terminating NUL character, then it may
      result in an out-of-bounds read.
      
      Fix the issue by using `git__strndup`, as was already done in the other
      location. Furthermore, add allocation checks to both locations to detect
      out-of-memory situations.
      Patrick Steinhardt committed
    • patch_parse: reject empty path names · 6c6c15e9
      When parsing patch headers, we currently accept empty path names just
      fine, e.g. a line "--- \n" would be parsed as the empty filename. This
      is not a valid patch format and may cause `NULL` pointer accesses at a
      later place as `git_buf_detach` will return `NULL` in that case.
      
      Reject such patches as malformed with a nice error message.
      Patrick Steinhardt committed
    • patch_parse: reject patches with multiple old/new paths · 223e7e43
      It's currently possible to have patches with multiple old path name
      headers. As we didn't check for this case, this resulted in a memory
      leak when overwriting the old old path with the new old path because we
      simply discarded the old pointer.
      
      Instead of fixing this by free'ing the old pointer, we should reject
      such patches altogether. It doesn't make any sense for the "---" or
      "+++" markers to occur multiple times within a patch n the first place.
      This also implicitly fixes the memory leak.
      Patrick Steinhardt committed
  10. 18 Oct, 2019 6 commits
  11. 17 Oct, 2019 9 commits
  12. 16 Oct, 2019 2 commits
  13. 13 Oct, 2019 4 commits