1. 11 Oct, 2018 3 commits
    • fuzzers: add object parsing fuzzer · a1d5fd06
      Add a simple fuzzer that exercises our object parser code. The fuzzer
      is quite trivial in that it simply passes the input data directly to
      `git_object__from_raw` for each of the four object types.
      Patrick Steinhardt committed
    • object: properly propagate errors on parsing failures · 6562cdda
      When failing to parse a raw object fromits data, we free the
      partially parsed object but then fail to propagate the error to the
      caller. This may lead callers to operate on objects with invalid memory,
      which will sooner or later cause the program to segfault.
      
      Fix the issue by passing up the error code returned by `parse_raw`.
      Patrick Steinhardt committed
    • fuzzers: initialize libgit2 in standalone driver · 6956a954
      The standalone driver for libgit2's fuzzing targets makes use of
      functions from libgit2 itself. While this is totally fine to do, we need
      to make sure to always have libgit2 initialized via `git_libgit2_init`
      before we call out to any of these. While this happens in most cases as
      we call `LLVMFuzzerInitialize`, which is provided by our fuzzers and
      which right now always calls `git_libgit2_init`, one exception to this
      rule is our error path when not enough arguments have been given. In
      this case, we will call `git_vector_free_deep` without libgit2 having
      been initialized. As we did not set up our allocation functions in that
      case, this will lead to a segmentation fault.
      
      Fix the issue by always initializing and shutting down libgit2 in the
      standalone driver. Note that we cannot let this replace the
      initialization in `LLVMFuzzerInitialize`, as it is required when using
      the "real" fuzzers by LLVM without our standalone driver. It's no
      problem to call the initialization and deinitialization functions
      multiple times, though.
      Patrick Steinhardt committed
  2. 05 Oct, 2018 10 commits
  3. 04 Oct, 2018 5 commits
  4. 03 Oct, 2018 12 commits
    • smart_pkt: do not accept callers passing in no line length · 1bc5b05c
      Right now, we simply ignore the `linelen` parameter of
      `git_pkt_parse_line` in case the caller passed in zero. But in fact, we
      never want to assume anything about the provided buffer length and
      always want the caller to pass in the available number of bytes.
      And in fact, checking all the callers, one can see that the funciton is
      never being called in case where the buffer length is zero, and thus we
      are safe to remove this check.
      Patrick Steinhardt committed
    • smart_pkt: return parsed length via out-parameter · c05790a8
      The `parse_len` function currently directly returns the parsed length of
      a packet line or an error code in case there was an error. Instead,
      convert this to our usual style of using the return value as error code
      only and returning the actual value via an out-parameter. Thus, we can
      now convert the output parameter to an unsigned type, as the size of a
      packet cannot ever be negative.
      
      While at it, we also move the check whether the input buffer is long
      enough into `parse_len` itself. We don't really want to pass around
      potentially non-NUL-terminated buffers to functions without also passing
      along the length, as this is dangerous in the unlikely case where other
      callers for that function get added. Note that we need to make sure
      though to not mess with `GIT_EBUFS` error codes, as these indicate not
      an error to the caller but that he needs to fetch more data.
      Patrick Steinhardt committed
    • smart_pkt: reorder and rename parameters of `git_pkt_parse_line` · 0b3dfbf4
      The parameters of the `git_pkt_parse_line` function are quite confusing.
      First, there is no real indicator what the `out` parameter is actually
      all about, and it's not really clear what the `bufflen` parameter refers
      to. Reorder and rename the parameters to make this more obvious.
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "unpack" packets · 5fabaca8
      When checking whether an "unpack" packet returned the "ok" status or
      not, we use a call to `git__prefixcmp`. In case where the passed line
      isn't properly NUL terminated, though, this may overrun the line buffer.
      Fix this by using `git__prefixncmp` instead.
      Patrick Steinhardt committed
    • smart_pkt: fix "ng" parser accepting non-space character · b5ba7af2
      When parsing "ng" packets, we blindly assume that the character
      immediately following the "ng" prefix is a space and skip it. As the
      calling function doesn't make sure that this is the case, we can thus
      end up blindly accepting an invalid packet line.
      
      Fix the issue by using `git__prefixncmp`, checking whether the line
      starts with "ng ".
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ok" packets · a9f1ca09
      There are two different buffer overflows present when parsing "ok"
      packets. First, we never verify whether the line already ends after
      "ok", but directly go ahead and also try to skip the expected space
      after "ok". Second, we then go ahead and use `strchr` to scan for the
      terminating newline character. But in case where the line isn't
      terminated correctly, this can overflow the line buffer.
      
      Fix the issues by using `git__prefixncmp` to check for the "ok " prefix
      and only checking for a trailing '\n' instead of using `memchr`. This
      also fixes the issue of us always requiring a trailing '\n'.
      
      Reported by oss-fuzz, issue 9749:
      
      Crash Type: Heap-buffer-overflow READ {*}
      Crash Address: 0x6310000389c0
      Crash State:
        ok_pkt
        git_pkt_parse_line
        git_smart__store_refs
      
      Sanitizer: address (ASAN)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ACK" packets · bc349045
      We are being quite lenient when parsing "ACK" packets. First, we didn't
      correctly verify that we're not overrunning the provided buffer length,
      which we fix here by using `git__prefixncmp` instead of
      `git__prefixcmp`. Second, we do not verify that the actual contents make
      any sense at all, as we simply ignore errors when parsing the ACKs OID
      and any unknown status strings. This may result in a parsed packet
      structure with invalid contents, which is being silently passed to the
      caller. This is being fixed by performing proper input validation and
      checking of return codes.
      Patrick Steinhardt committed
    • smart_pkt: adjust style of "ref" packet parsing function · 5edcf5d1
      While the function parsing ref packets doesn't have any immediately
      obvious buffer overflows, it's style is different to all the other
      parsing functions. Instead of checking buffer length while we go, it
      does a check up-front. This causes the code to seem a lot more magical
      than it really is due to some magic constants. Refactor the function to
      instead make use of the style of other packet parser and verify buffer
      lengths as we go.
      Patrick Steinhardt committed
    • smart_pkt: check whether error packets are prefixed with "ERR " · 786426ea
      In the `git_pkt_parse_line` function, we determine what kind of packet
      a given packet line contains by simply checking for the prefix of that
      line. Except for "ERR" packets, we always only check for the immediate
      identifier without the trailing space (e.g. we check for an "ACK"
      prefix, not for "ACK "). But for "ERR" packets, we do in fact include
      the trailing space in our check. This is not really much of a problem at
      all, but it is inconsistent with all the other packet types and thus
      causes confusion when the `err_pkt` function just immediately skips the
      space without checking whether it overflows the line buffer.
      
      Adjust the check in `git_pkt_parse_line` to not include the trailing
      space and instead move it into `err_pkt` for consistency.
      Patrick Steinhardt committed
    • smart_pkt: explicitly avoid integer overflows when parsing packets · 40fd84cc
      When parsing data, progress or error packets, we need to copy the
      contents of the rest of the current packet line into the flex-array of
      the parsed packet. To keep track of this array's length, we then assign
      the remaining length of the packet line to the structure. We do have a
      mismatch of types here, as the structure's `len` field is a signed
      integer, while the length that we are assigning has type `size_t`.
      
      On nearly all platforms, this shouldn't pose any problems at all. The
      line length can at most be 16^4, as the line's length is being encoded
      by exactly four hex digits. But on a platforms with 16 bit integers,
      this assignment could cause an overflow. While such platforms will
      probably only exist in the embedded ecosystem, we still want to avoid
      this potential overflow. Thus, we now simply change the structure's
      `len` member to be of type `size_t` to avoid any integer promotion.
      Patrick Steinhardt committed
    • smart_pkt: honor line length when determining packet type · 4a5804c9
      When we parse the packet type of an incoming packet line, we do not
      verify that we don't overflow the provided line buffer. Fix this by
      using `git__prefixncmp` instead and passing in `len`. As we have
      previously already verified that `len <= linelen`, we thus won't ever
      overflow the provided buffer length.
      Patrick Steinhardt committed
    • tests: verify parsing logic for smart packets · 365d2720
      The commits following this commit are about to introduce quite a lot of
      refactoring and tightening of the smart packet parser. Unfortunately, we
      do not yet have any tests despite our online tests that verify that our
      parser does not regress upon changes. This is doubly unfortunate as our
      online tests aren't executed by default.
      
      Add new tests that exercise the smart parsing logic directly by
      executing `git_pkt_parse_line`.
      Patrick Steinhardt committed
  5. 01 Oct, 2018 3 commits
  6. 29 Sep, 2018 3 commits
  7. 28 Sep, 2018 4 commits
    • Merge pull request #4767 from pks-t/pks/config-mem · 0530d7d9
      In-memory configuration
      Carlos Martín Nieto committed
    • config: introduce new read-only in-memory backend · 2be39cef
      Now that we have abstracted away how to store and retrieve config
      entries, it became trivial to implement a new in-memory backend by
      making use of this. And thus we do so.
      
      This commit implements a new read-only in-memory backend that can parse
      a chunk of memory into a `git_config_backend` structure.
      Patrick Steinhardt committed
    • config_entries: refactor entries iterator memory ownership · b78f4ab0
      Right now, the config file code requires us to pass in its backend to
      the config entry iterator. This is required with the current code, as
      the config file backend will first create a read-only snapshot which is
      then passed to the iterator just for that purpose. So after the iterator
      is getting free'd, the code needs to make sure that the snapshot gets
      free'd, as well.
      
      By now, though, we can easily refactor the code to be more efficient and
      remove the reverse dependency from iterator to backend. Instead of
      creating a read-only snapshot (which also requires us to re-parse the
      complete configuration file), we can simply duplicate the config entries
      and pass those to the iterator. Like that, the iterator only needs to
      make sure to free the duplicated config entries, which is trivial to do
      and clears up memory ownership by a lot.
      Patrick Steinhardt committed
    • config_entries: internalize structure declarations · d49b1365
      Access to the config entries is now completely done via the modules
      function interface and no caller messes with the struct's internals. We
      can thus completely move the structure declarations into the
      implementation file so that nobody even has a chance to mess with the
      members.
      Patrick Steinhardt committed