- 02 Nov, 2018 4 commits
-
-
When parsing a tree entry's mode, we will eagerly parse until we hit a character that is not in the accepted set of octal digits '0' - '7'. If the provided buffer is not a NUL terminated one, we may thus read out-of-bounds. Fix the issue by passing the buffer length to `parse_mode` and paying attention to it. Note that this is not a vulnerability in our usual code paths, as all object data read from the ODB is NUL terminated.
Patrick Steinhardt committed -
We currently don't have any tests that directly exercise the tree parser. This is due to the fact that the parsers for raw object data has only been recently introduce with commit ca4db5f4 (object: implement function to parse raw data, 2017-10-13), and previous to that the setup simply was too cumbersome as it always required going through the ODB. Now that we have the infrastructure, add a suite of tests that directly exercise the tree parser and various edge cases.
Patrick Steinhardt committed -
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
-
- 31 Oct, 2018 1 commit
-
-
CI: Fix macOS leak detection
Edward Thomson committed
-
- 30 Oct, 2018 2 commits
-
-
Etienne Samson committed
-
Etienne Samson committed
-
- 26 Oct, 2018 6 commits
-
-
README: more CI status badges
Edward Thomson committed -
ci: Fix some minor issues
Edward Thomson committed -
Don't prefix the path to the yaml templates - the nightly template itself is already in the `azure-pipelines` directory. Instead, just use the relative path.
Edward Thomson committed -
POSIX: the CMakeLists.txt configures the test names; when we query ctest for the test command-line to run, fail if the tests are not found.
Edward Thomson committed -
Win32: The CMakeLists.txt configures the test names; when we query ctest for the test command-line to run, fail if the tests are not found.
Edward Thomson committed -
Object parse fixes
Patrick Steinhardt committed
-
- 25 Oct, 2018 11 commits
-
-
Windows CI: fail build on test failure
Edward Thomson committed -
ci: run all the jobs during nightly builds
Edward Thomson committed -
Instead of running the oddball builds, run all the builds (the ones that we always run during PR validation and CI) during a nightly build for increased coverage.
Edward Thomson committed -
PowerShell can _read_ top-level variables in functions, but cannot _update_ top-level variables in functions unless they're explicitly prefixed with `$global`.
Edward Thomson 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.
Patrick Steinhardt committed -
We currently do not have any test suites dedicated to parsing commits from their raw representations. Add one based on `git_object__from_raw` to be able to test special cases more easily.
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.
Patrick Steinhardt committed -
While the tests in object::tag::read exercises reading and parsing valid tags from the ODB, they barely try to verify that the parser fails in a sane way when parsing invalid tags. Create a new test suite object::tag::parse that directly exercise the parser by using `git_object__from_raw` and add various tests for valid and invalid tags.
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/
Patrick Steinhardt committed -
strtol removal
Patrick Steinhardt committed -
buf::oom tests: use custom allocator for oom failures
Patrick Steinhardt committed
-
- 23 Oct, 2018 1 commit
-
-
Etienne Samson committed
-
- 21 Oct, 2018 12 commits
-
-
Create a custom allocator for the `buf::oom` tests that will fail with out-of-memory errors in predictable ways. We were previously trying to guess the way that various allocators on various platforms would fail in a way such that `malloc`/`realloc` would return `NULL` (instead of aborting the application, or appearing suspicious to various instrumentation or static code analysis tools like valgrind.) Introduce a fake `malloc` and `realloc` that will return `NULL` on allocations requesting more than 100 bytes. Otherwise, we proxy to the default allocator. (It's important to use the _default_ allocator, not just call `malloc`, since the default allocator on Windows CI builds may be the debugging C runtime allocators which would not be compatible with a standard `malloc`.)
Edward Thomson committed -
Provide a utility to reset custom allocators back to their default. This is particularly useful for testing.
Edward Thomson committed -
ci: arm docker builds
Edward Thomson committed -
We don't need two separate docker images for OpenSSL and mbedTLS. They've been combined into a single image `trusty-amd64` that supports both.
Edward Thomson committed -
On a 32-bit Linux systems, the value large enough to make malloc guarantee a failure is also large enough that valgrind considers it "fishy". Skip this test on those systems entirely.
Edward Thomson committed -
Newer dependencies means newer places to leak!
Edward Thomson committed -
Use Bionic so that we have a modern libssh2 (for communicating with GitHub). We've ported fixes to our Trusty-based amd64 images, but maintaining patches for multiple platforms is heinous.
Edward Thomson committed -
Edward Thomson committed
-
Bind the proxy specifically to 127.0.0.1 instead of all addresses. This is not strictly necessary for operations, but having a potentially open proxy on a network is not a good idea.
Edward Thomson committed -
Use multiarch arm32 and arm64 docker images to run Xenial-based images for those platforms. We can support all the tests on ARM32 and 64 _except_ the proxy-based tests. Our proxy on ARM seems regrettably unstable, either due to some shoddy dependencies (with native code?) or the JREs themselves. Run these platforms as part of our nightly builds; do not run them during pull request or CI validation.
Edward Thomson committed -
Edward Thomson committed
-
As the number of each grow, separate the CI build scripts from the YAML definitions.
Edward Thomson committed
-
- 20 Oct, 2018 3 commits
-
-
Win32 path canonicalization refactoring
Edward Thomson committed -
Check object existence when creating a tree from an index
Edward Thomson committed -
Edward Thomson committed
-