- 26 Oct, 2018 40 commits
-
-
Previously we would assert in index_free because the reader incrementation would not be balanced. Release the snapshot normally, so the variable gets decremented before the index is freed. (cherry picked from commit c70713d6)
Etienne Samson committed -
While most systems provide a separate iconv library against which applications can link, musl based systems do not provide such a library. Instead, iconv functions are directly included in the C library. As our current CMake module to locate the iconv library only checks whether a library exists somewhere in the typical library directories, we will never build libgit2 with libiconv support on such systems. Extend the iconv module to also search whether libc provides iconv functions, which we do by checking whether the `iconv_open` function exists inside of libc. If this is the case, we will default to use the libc provided one instead of trying to use a separate libiconv. While this changes which iconv we use on systems where both libc and an external libiconv exist, to the best of my knowledge common systems only provide either one or the other. Note that libiconv support in musl is held kind of basic. To quote musl libc's page on functional differences from glibc [1]: The iconv implementation musl is very small and oriented towards being unobtrusive to static link. Its character set/encoding coverage is very strong for its size, but not comprehensive like glibc’s. As we assume iconv to be a lot more capable than what musl provides, some of our tests will fail if using iconv on musl-based platforms. [1]: https://wiki.musl-libc.org/functional-differences-from-glibc.html (cherry picked from commit 2e2d8c64)
Patrick Steinhardt committed -
Otherwise we'll return stack data to the caller. (cherry picked from commit 22d013b6)
Etienne Samson committed -
At line 594, we do this : if (error < 0) return error; but if nothing was pushed in a GIT_SORT_TIME revwalk, we'd return uninitialized stack data. (cherry picked from commit aa8cb586)
Etienne Samson committed -
The documentation states that git_worktree_unlock returns 0 on success, and 1 on success if the worktree wasn't locked. Turns out we were returning 0 in any of those cases. (cherry picked from commit 59c2e70e)
Etienne Samson committed -
This performs a compile-check by using CMake support, to differentiate the GNU version from the BSD version of qsort_r. Module taken from 4f252abea5f1d17c60f6ff115c9c44cc0b6f1df6, which I've checked against CMake 2.8.11. (cherry picked from commit 1a9cc182)
Etienne Samson committed -
The function `git_diff_find_similar` keeps a function of cache similarity metrics signatures, whose size depends on the number of deltas passed in via the `diff` parameter. In case where the diff is empty and thus doesn't have any deltas at all, we may end up allocating this cache via a call to `git__calloc(0, sizeof(void *))`. At least on AIX, allocating 0 bytes will result in a `NULL` pointer being returned, which causes us to erroneously return an OOM error. Fix this situation by simply returning early in case where we are being passed an empty diff, as we cannot find any similarities in that case anyway. (cherry picked from commit c65568d8)
Patrick Steinhardt committed -
(cherry picked from commit 6698e056)
Nelson Elhage committed -
(cherry picked from commit f1409500)
Nelson Elhage committed -
(cherry picked from commit ec76a1aa)
Nelson Elhage committed -
(cherry picked from commit 019409be)
Nelson Elhage committed -
(cherry picked from commit c4d7fa95)
Nelson Elhage committed -
String operations in libgit2 are supposed to never receive `NULL`, e.g. they are not `NULL`-save. In the case of `git__linenlen()`, invocation with `NULL` leads to undefined behavior. In a `git_parse_ctx` however, the `content` field used in these operations was initialized to `NULL` if the `git_parse_ctx_init()` was called with `NULL` for `content` or `0` for `content_len`. For the latter case, the initialization function even contained some logic for initializing `content` with `NULL`. This commit mitigates triggering undefined behavior by rewriting the logic. Now `content` is always initialized to a non-null buffer. Instead of a null buffer, an empty string is used for denoting an empty buffer. (cherry picked from commit d1bfe614)
Julian Ganz committed -
(cherry picked from commit f00db9ed)
Carlos Martín Nieto committed -
When we add entries to a treebuilder we validate them. But we validate even those that we're adding because they exist in the base tree. This disables using the normal mechanisms on these trees, even to fix them. Keep track of whether the entry we're appending comes from an existing tree and bypass the name and id validation if it's from existing data. (cherry picked from commit 2dff7e28)
Carlos Martín Nieto committed -
(cherry picked from commit f9e28026)
Etienne Samson committed -
Etienne Samson committed
-
We were previously conflating any error into GIT_ENOTFOUND, which might or might not be correct. This fixes the code so a config error is bubbled up, as well as preserving the semantics in the face of worktree-repositories
Etienne Samson committed -
Release v0.27.6
Patrick Steinhardt committed -
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
The commit message encoding is currently being parsed by the `git__prefixcmp` function. As this function does not accept a buffer length, it will happily skip over a buffer's end if it is not `NUL` terminated. Fix the issue by using `git__prefixncmp` instead. Add a test that verifies that we are unable to parse the encoding field if it's cut off by the supplied buffer length. (cherry picked from commit 7655b2d8)
Patrick Steinhardt committed -
When parsing tags, we skip all unknown fields that appear before the tag message. This skipping is done by using a plain `strstr(buffer, "\n\n")` to search for the two newlines that separate tag fields from tag message. As it is not possible to supply a buffer length to `strstr`, this call may skip over the buffer's end and thus result in an out of bounds read. As `strstr` may return a pointer that is out of bounds, the following computation of `buffer_end - buffer` will overflow and result in an allocation of an invalid length. Fix the issue by using `git__memmem` instead. Add a test that verifies parsing the tag fails not due to the allocation failure but due to the tag having no message. (cherry picked from commit ee11d47e)
Patrick Steinhardt committed -
Unfortunately, neither the `memmem` nor the `strnstr` functions are part of any C standard but are merely extensions of C that are implemented by e.g. glibc. Thus, there is no standardized way to search for a string in a block of memory with a limited size, and using `strstr` is to be considered unsafe in case where the buffer has not been sanitized. In fact, there are some uses of `strstr` in exactly that unsafe way in our codebase. Provide a new function `git__memmem` that implements the `memmem` semantics. That is in a given haystack of `n` bytes, search for the occurrence of a byte sequence of `m` bytes and return a pointer to the first occurrence. The implementation chosen is the "Not So Naive" algorithm from [1]. It was chosen as the implementation is comparably simple while still being reasonably efficient in most cases. Preprocessing happens in constant time and space, searching has a time complexity of O(n*m) with a slightly sub-linear average case. [1]: http://www-igm.univ-mlv.fr/~lecroq/string/ (cherry picked from commit 83e8a6b3)
Patrick Steinhardt committed -
When an integer that is parsed with `git__strntol32` is too big to fit into an int32, we will generate an error message that includes the actual string that failed to parse. This does not acknowledge the fact that the string may either not be NUL terminated or alternative include additional characters after the number that is to be parsed. We may thus end up printing characters into the buffer that aren't the number or, worse, read out of bounds. Fix the issue by utilizing the `endptr` that was set by `git__strntol64`. This pointer is guaranteed to be set to the first character following the number, and we can thus use it to compute the width of the number that shall be printed. Create a test to verify that we correctly truncate the number. (cherry picked from commit ea19efc1)
Patrick Steinhardt committed -
While `git__strntol64` tries to detect integer overflows when doing the necessary arithmetics to come up with the final result, it does the detection only after the fact. This check thus relies on undefined behavior of signed integer overflows. Fix this by instead checking up-front whether the multiplications or additions will overflow. Note that a detected overflow will not cause us to abort parsing the current sequence of digits. In the case of an overflow, previous behavior was to still set up the end pointer correctly to point to the first character immediately after the currently parsed number. We do not want to change this now as code may rely on the end pointer being set up correctly even if the parsed number is too big to be represented as 64 bit integer. (cherry picked from commit b09c1c7b)
Patrick Steinhardt committed -
Some edge cases were currently completely untested, e.g. parsing numbers greater than INT64_{MIN,MAX}, truncating buffers by length and invalid characters. Add tests to verify that the system under test performs as expected. (cherry picked from commit 39087ab8)
Patrick Steinhardt committed -
The function `git__strtol32` can easily be misused when untrusted data is passed to it that may not have been sanitized with trailing `NUL` bytes. As all usages of this function have now been removed, we can remove this function altogether to avoid future misuse of it. (cherry picked from commit 8d7fa88a)
Patrick Steinhardt committed -
Replace remaining uses of the `git__strtol32` function. While these uses are all safe as the strings were either sanitized or from a trusted source, we want to remove `git__strtol32` altogether to avoid future misuse. (cherry picked from commit 2613fbb2)
Patrick Steinhardt committed -
We use the `git__strtol32` function to parse the child and entry count of treecaches from the index, which do not accept a buffer length. As the buffer that is being passed in is untrusted data and may thus be malformed and may not contain a terminating `NUL` byte, we can overrun the buffer and thus perform an out-of-bounds read. Fix the issue by uzing `git__strntol32` instead. (cherry picked from commit 21652ee9)
Patrick Steinhardt committed -
The function `git__strtol64` does not take a maximum buffer length as parameter. This has led to some unsafe usages of this function, and as such we may consider it as being unsafe to use. As we have now eradicated all usages of this function, let's remove it completely to avoid future misuse. (cherry picked from commit 68deb2cc)
Patrick Steinhardt committed -
When parsing integers from configuration values, we use `git__strtol64`. This is fine to do, as we always sanitize values and can thus be sure that they'll have a terminating `NUL` byte. But as this is the last call-site of `git__strtol64`, let's just pass in the length explicitly by calling `strlen` on the value to be able to remove `git__strtol64` altogether. (cherry picked from commit 1a2efd10)
Patrick Steinhardt committed -
We use `git__strtol64` and `git__strtol32` to parse the trailing commit or author date and timezone of signatures. As signatures are usually part of a commit or tag object and thus essentially untrusted data, the buffer may be misformatted and may not be `NUL` terminated. This may lead to an out-of-bounds read. Fix the issue by using `git__strntol64` and `git__strntol32` instead. (cherry picked from commit 3db9aa6f)
Patrick Steinhardt committed -
We use `git__strtol64` to parse file modes of the index entries, which does not limit the parsed buffer length. As the index can be essentially treated as "untrusted" in that the data stems from the file system, it may be misformatted and may not contain terminating `NUL` bytes. This may lead to out-of-bounds reads when trying to parse index entries with such malformatted modes. Fix the issue by using `git__strntol64` instead. (cherry picked from commit 600ceadd)
Patrick Steinhardt committed -
When quick-parsing a commit, we use `git__strtol64` to parse the commit's time. The buffer that's passed to `commit_quick_parse` is the raw data of an ODB object, though, whose data may not be properly formatted and also does not have to be `NUL` terminated. This may lead to out-of-bound reads. Use `git__strntol64` to avoid this problem. (cherry picked from commit 1a3fa1f5)
Patrick Steinhardt committed -
Before resetting the url and username, ensure that we free them in case they were set by environment variables. (cherry picked from commit e84914fd)
Edward Thomson committed -
Don't just free the spec vector, also free the specs themselves. (cherry picked from commit d285de73)
Edward Thomson committed -
Don't just free the push status structure, actually free the strings that were strdup'd into the struct as well. (cherry picked from commit dad99881)
Edward Thomson committed -
Free the url field when resetting the stream to avoid leaking it. (cherry picked from commit ca2eb460)
Edward Thomson committed
-