1. 20 Oct, 2018 1 commit
    • checkout: fix test fixture missing objects · c79e6081
      The testrepo test fixture has an index file that's damaged, missing an
      object.  The index previously had an entry of `src/index.c` with id
      3161df8cbf3a006b4ef85be6497a0ea6bde98541, but that object was missing in
      the repository.  This commit adds an object to the repository and
      updates the index to use that existing blob.
      
      Similarly, the index has an entry for `readme` with an id of
      97328ac7e3bd0bcd3900cb3e7a624d71dd0df888.  This can be restored from
      other test repositories.
      
      With these fixed, now the write tree from index tests can pass since they
      validate object existence.
      Edward Thomson committed
  2. 08 Oct, 2018 2 commits
    • tree: unify the entry validity checks · fd490d3e
      We have two similar functions, `git_treebuilder_insert` and `append_entry` which
      are used in different codepaths as part of creating a new tree. The former
      learnt to check for object existence under strict object creation, but the
      latter did not.
      
      This allowed the creation of a tree from an unowned index to bypass some of the
      checks and create a tree pointing to a nonexistent object.
      
      Extract a single function which performs these checks and call it from both
      codepaths. In `append_entry` we still do not validate when asked not to, as this
      is data which is already in the tree and we want to allow users to deal with
      repositories which already have some invalid data.
      Carlos Martín Nieto committed
    • index: add failing test for writing an invalid tree from an unowned index · fbc0dcda
      When the index does not belong to any repository, we do not do any checks of the
      target id going in as we cannot verify that it exists.
      
      When we then write it out to a repository as a tree, we fail to perform the
      object existance and type-matching check that we do in other code-paths. This
      leads to being able to write trees which point to non-existent blobs even with
      strict object creation enabled.
      Carlos Martín Nieto committed
  3. 07 Oct, 2018 3 commits
  4. 06 Oct, 2018 1 commit
    • ignore unsupported http authentication schemes · 475db39b
      auth_context_match returns 0 instead of -1 for unknown schemes to
      not fail in situations where some authentication schemes are supported
      and others are not.
      
      apply_credentials is adjusted to handle auth_context_match returning
      0 without producing authentication context.
      Anders Borum committed
  5. 05 Oct, 2018 12 commits
  6. 04 Oct, 2018 7 commits
  7. 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
  8. 01 Oct, 2018 2 commits