- 28 Nov, 2018 3 commits
-
-
Accept an enum (`git_stream_t`) during custom stream registration that indicates whether the registration structure should be used for standard (non-TLS) streams or TLS streams.
Edward Thomson committed -
Update the new stream registration API to be `git_stream_register` which takes a registration structure and a TLS boolean. This allows callers to register non-TLS streams as well as TLS streams. Provide `git_stream_register_tls` that takes just the init callback for backward compatibliity.
Edward Thomson committed -
Introduce `git_tls_stream_wrap` which will take an existing `stream` with an already connected socket and begin speaking TLS on top of it. This is useful if you've built a connection to a proxy server and you wish to begin CONNECT over it to tunnel a TLS connection. Also update the pluggable TLS stream layer so that it can accept a registration structure that provides an `init` and `wrap` function, instead of a single initialization function.
Edward Thomson committed
-
- 14 Nov, 2018 1 commit
-
-
When parsing a number, we accept a leading plus or minus sign to return a positive or negative number. When the parsed string has such a leading sign, we set up a flag indicating that the number is negative and advance the pointer to the next character in that string. This misses updating the number of bytes in the string, though, which is why the parser may later on do an out-of-bounds read. Fix the issue by correctly updating both the pointer and the number of remaining bytes. Furthermore, we need to check whether we actually have any bytes left after having advanced the pointer, as otherwise the auto-detection of the base may do an out-of-bonuds access. Add a test that detects the out-of-bound read. Note that this is not actually security critical. While there are a lot of places where the function is called, all of these places are guarded or irrelevant: - commit list: this operates on objects from the ODB, which are always NUL terminated any may thus not trigger the off-by-one OOB read. - config: the configuration is NUL terminated. - curl stream: user input is being parsed that is always NUL terminated - index: the index is read via `git_futils_readbuffer`, which always NUL terminates it. - loose objects: used to parse the length from the object's header. As we check previously that the buffer contains a NUL byte, this is safe. - rebase: this parses numbers from the rebase instruction sheet. As the rebase code uses `git_futils_readbuffer`, the buffer is always NUL terminated. - revparse: this parses a user provided buffer that is NUL terminated. - signature: this parser the header information of objects. As objects read from the ODB are always NUL terminated, this is a non-issue. The constructor `git_signature_from_buffer` does not accept a length parameter for the buffer, so the buffer needs to be NUL terminated, as well. - smart transport: the buffer that is parsed is NUL terminated - tree cache: this parses the tree cache from the index extension. The index itself is read via `git_futils_readbuffer`, which always NUL terminates it. - winhttp transport: user input is being parsed that is always NUL terminated
Patrick Steinhardt committed
-
- 02 Nov, 2018 2 commits
-
-
The `git__strntol` family of functions has the ability to auto-detect a number's base if the string has either the common '0x' prefix for hexadecimal numbers or '0' prefix for octal numbers. The detection of such prefixes and following handling has two major issues though that are being fixed in one go now. - We do not do any bounds checking previous to verifying the '0x' base. While we do verify that there is at least one digit available previously, we fail to verify that there are two digits available and thus may do an out-of-bounds read when parsing this two-character-prefix. - When skipping the prefix of such numbers, we only update the pointer length without also updating the number of remaining bytes. Thus if we try to parse a number '0x1' of total length 3, we will first skip the first two bytes and then try to read 3 bytes starting at '1'. Fix both issues by disentangling the logic. Instead of doing the detection and skipping of such prefixes in one go, we will now first try to detect the base while also honoring how many bytes are left. Only if we have a valid base that is either 8 or 16 and have one of the known prefixes, we will now advance the pointer and update the remaining bytes in one step. Add some tests that verify that no out-of-bounds parsing happens and that autodetection works as advertised.
Patrick Steinhardt committed -
The `git__strntol` family of functions accepts leading spaces and will simply skip them. The skipping will not honor the provided buffer's length, though, which may lead it to read outside of the provided buffer's bounds if it is not a simple NUL-terminated string. Furthermore, if leading space is trimmed, the function will further advance the pointer but not update the number of remaining bytes, which may also lead to out-of-bounds reads. Fix the issue by properly paying attention to the buffer length and updating it when stripping leading whitespace characters. Add a test that verifies that we won't read past the provided buffer length.
Patrick Steinhardt committed
-
- 25 Oct, 2018 1 commit
-
-
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/
Patrick Steinhardt committed
-
- 19 Oct, 2018 1 commit
-
-
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.
Patrick Steinhardt committed
-
- 18 Oct, 2018 3 commits
-
-
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.
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.
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.
Patrick Steinhardt committed
-
- 05 Oct, 2018 3 commits
-
-
Currently, we do not clean up after ourselves after tests in core::rmdir have created new files in the directory hierarchy. This may leave stale files and/or directories after having run tests, confusing subsequent tests that expect a pristine test environment. Most importantly, it may cause the test initialization to fail which expects being able to re-create the testing hierarchy before each test in case where another test hasn't cleaned up after itself. Fix the issue by adding a cleanup function that removes the temporary testing hierarchy after each test if it still exists.
Patrick Steinhardt committed -
Signed-off-by: Sven Strickroth <email@cs-ware.de>
Sven Strickroth committed -
GCC warns by default when implicitly converting integers to pointers or the other way round, and commit fa48d2ea (vector: do not malloc 0-length vectors on dup, 2018-09-26) introduced such an implicit conversion into our vector tests. While this is totally fine in this test, as the pointer's value is never being used in the first place, we can trivially avoid the warning by instead just inserting a pointer for a variable allocated on the stack into the vector.
Patrick Steinhardt committed
-
- 26 Sep, 2018 1 commit
-
-
Etienne Samson committed
-
- 25 Sep, 2018 1 commit
-
-
Etienne Samson committed
-
- 13 Jul, 2018 1 commit
-
-
C++ style comment ("//") are not specified by the ISO C90 standard and thus do not conform to it. While libgit2 aims to conform to C90, we did not enforce it until now, which is why quite a lot of these non-conforming comments have snuck into our codebase. Do a tree-wide conversion of all C++ style comments to the supported C style comments to allow us enforcing strict C90 compliance in a later commit.
Patrick Steinhardt committed
-
- 10 Jun, 2018 1 commit
-
-
Patrick Steinhardt committed
-
- 11 Apr, 2018 1 commit
-
-
Etienne Samson committed
-
- 20 Dec, 2017 1 commit
-
-
Introduce `git_prefixncmp` that will search up to the first `n` characters of a string to see if it is prefixed by another string. This is useful for examining if a non-null terminated character array is prefixed by a particular substring. Consolidate the various implementations of `git__prefixcmp` around a single core implementation and add some test cases to validate its behavior.
Edward Thomson committed
-
- 23 Oct, 2017 2 commits
-
-
Etienne Samson committed
-
Etienne Samson committed
-
- 26 Jun, 2017 1 commit
-
-
The upstream git project provides the ability to calculate a so-called patch ID. Quoting from git-patch-id(1): A "patch ID" is nothing but a sum of SHA-1 of the file diffs associated with a patch, with whitespace and line numbers ignored." Patch IDs can be used to identify two patches which are probably the same thing, e.g. when a patch has been cherry-picked to another branch. This commit implements a new function `git_diff_patchid`, which gets a patch and derives an OID from the diff. Note the different terminology here: a patch in libgit2 are the differences in a single file and a diff can contain multiple patches for different files. The implementation matches the upstream implementation and should derive the same OID for the same diff. In fact, some code has been directly derived from the upstream implementation. The upstream implementation has two different modes to calculate patch IDs, which is the stable and unstable mode. The old way of calculating the patch IDs was unstable in a sense that a different ordering the diffs was leading to different results. This oversight was fixed in git 1.9, but as git tries hard to never break existing workflows, the old and unstable way is still default. The newer and stable way does not care for ordering of the diff hunks, and in fact it is the mode that should probably be used today. So right now, we only implement the stable way of generating the patch ID.
Patrick Steinhardt committed
-
- 13 Jun, 2017 2 commits
-
-
Mohseen Mukaddam committed
-
Initialization of the `git_proxy_options` structure is never tested anywhere. Include it in our usual initialization test in "core::structinit::compare".
Patrick Steinhardt committed
-
- 06 Jun, 2017 1 commit
-
-
When encoding varints to a buffer, we want to remain sure that the required buffer space does not exceed what is actually available. Our current check does not do the right thing, though, in that it does not honor that our `pos` variable counts the position down instead of up. As such, we will require too much memory for small varints and not enough memory for big varints. Fix the issue by correctly calculating the required size as `(sizeof(varint) - pos)`. Add a test which failed before.
Patrick Steinhardt committed
-
- 25 Apr, 2017 1 commit
-
-
Patrick Steinhardt committed
-
- 22 Mar, 2017 1 commit
-
-
Address family 5 might exist on some crazy system like Haiku. Use `INT_MAX-1` as an unsupported address family.
Edward Thomson committed
-
- 20 Mar, 2017 1 commit
-
-
Patrick Steinhardt committed
-
- 03 Mar, 2017 1 commit
-
-
Edward Thomson committed
-
- 25 Feb, 2017 1 commit
-
-
Haiku will assert in a nightly build if the "dst" input to inet_pton() is NULL.
Kevin Wojniak committed
-
- 17 Feb, 2017 8 commits
-
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
Patrick Steinhardt committed
-
- 08 Feb, 2017 1 commit
-
-
When calling `git_path_dirname_r` on a Win32 prefix, e.g. a drive or network share prefix, we always want to return the trailing '/'. This does not work currently when passing in a path like 'C:', where the '/' would not be appended correctly. Fix this by appending a '/' if we try to normalize a Win32 prefix and there is no trailing '/'.
Patrick Steinhardt committed
-