Skip to content

G
git2
Switch branch/tag
  1. 12 Oct, 2018 25 commits
  3. 05 Oct, 2018 7 commits
  5. 03 Oct, 2018 8 commits
    • smart_pkt: do not accept callers passing in no line length · f5c3442b 
      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.

(cherry picked from commit 1bc5b05c)
      Patrick Steinhardt committed
    • smart_pkt: return parsed length via out-parameter · f7c3f6cc 
      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.

(cherry picked from commit c05790a8)
      Patrick Steinhardt committed
    • smart_pkt: reorder and rename parameters of `git_pkt_parse_line` · 7e3cd611 
      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.

(cherry picked from commit 0b3dfbf4)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "unpack" packets · 356f60f4 
      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.

(cherry picked from commit 5fabaca8)
      Patrick Steinhardt committed
    • smart_pkt: fix "ng" parser accepting non-space character · b5b7c303 
      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 ".

(cherry picked from commit b5ba7af2)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ok" packets · 319f0c03 
      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)
(cherry picked from commit a9f1ca09)
      Patrick Steinhardt committed
    • smart_pkt: fix buffer overflow when parsing "ACK" packets · 0599c267 
      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.

(cherry picked from commit bc349045)
      Patrick Steinhardt committed
    • smart_pkt: adjust style of "ref" packet parsing function · 0fe87761 
      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.

(cherry picked from commit 5edcf5d1)
      Patrick Steinhardt committed