summaryrefslogtreecommitdiff
path: root/tests
Commit message (Collapse)AuthorAgeFilesLines
* util: provide `git__memmem` functionPatrick Steinhardt2018-10-261-0/+46
| | | | | | | | | | | | | | | | | | | | | | | 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 83e8a6b36acc67f2702cbbc7d4e334c7f7737719)
* util: fix out of bounds read in error messagePatrick Steinhardt2018-10-261-0/+7
| | | | | | | | | | | | | | | | | | 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 ea19efc19fa683d632af3e172868bc4350724813)
* tests: core::strtol: test for some more edge-casesPatrick Steinhardt2018-10-261-0/+31
| | | | | | | | | 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 39087ab8ef77004c9f3b8984c38a834a6cb238bc)
* util: remove `git__strtol32`Patrick Steinhardt2018-10-261-12/+19
| | | | | | | | | 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 8d7fa88a9d5011b653035497b0f523e0f177b6a6)
* util: remove unsafe `git__strtol64` functionPatrick Steinhardt2018-10-261-22/+16
| | | | | | | | | | 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 68deb2cc80ef19bf3a1915c26b5308b283a6d69a)
* online::clone: free url and username before resettingEdward Thomson2018-10-261-0/+6
| | | | | | | Before resetting the url and username, ensure that we free them in case they were set by environment variables. (cherry picked from commit e84914fd30edc6702e368c8ccfc77dc5607c213c)
* push tests: deeply free the specsEdward Thomson2018-10-261-1/+1
| | | | | | Don't just free the spec vector, also free the specs themselves. (cherry picked from commit d285de73f9a09bc841b329267d1f61b9c03a7b68)
* push tests: deeply free the push statusEdward Thomson2018-10-261-2/+6
| | | | | | | 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 dad9988121521ccc2ffff39299ca98dba160b857)
* tests: online::clone: fix memory leak due to not freeing URLPatrick Steinhardt2018-10-261-0/+2
| | | | (cherry picked from commit 820fb71292c8065ee83b00f82b909806916e0df2)
* clar: iterate errors in report_all / report_errorsEdward Thomson2018-10-261-19/+15
| | | | | | | | Instead of trying to have a clever iterator pattern that increments the error number, just iterate over errors in the report errors or report all functions as it's easier to reason about in this fashion. (cherry picked from commit d17e67d08d6e73dbf0daeae5049f92a38c2d8bb6)
* ci: use more compatible strftime formatsEdward Thomson2018-10-261-1/+1
| | | | | | | Windows lacks %F and %T formats for strftime. Expand them to the year/month/day and hour/minute/second formats, respectively. (cherry picked from commit e595eeb5ab88142b97798ed65e651de6560515e9)
* clar: remove globals; error-check fprintf/fcloseEdward Thomson2018-10-262-44/+87
| | | | | | | | Remove the global summary filename and file pointer; pass them in to the summary functions as needed. Error check the results of buffered I/O calls. (cherry picked from commit b67a93ff81e2fbfcf9ebb52dd15db9aa4e9ca708)
* clar: accept a value for the summary filenameEdward Thomson2018-10-262-12/+25
| | | | | | | Accept an (optional) value for the summary filename. Continues to default to summary.xml. (cherry picked from commit baa5c20d0815441cac2d2135d2b0190cb543e637)
* clar: don't use a variable named `time`Edward Thomson2018-10-261-4/+4
| | | | (cherry picked from commit dbebcb04b42047df0d52ad3515077a134c5b7da7)
* Barebones JUnit XML outputEtienne Samson2018-10-262-1/+111
| | | | (cherry picked from commit 59f1e477f772c73c76bc654a0853fdcf491a32a7)
* DocumentationEtienne Samson2018-10-261-0/+2
| | | | (cherry picked from commit 3a9b96311d6f0ff364c6417cf3aab7c9745b18d4)
* Isolate test reportsEtienne Samson2018-10-263-36/+82
| | | | | | | This makes it possible to keep track of every test status (even successful ones), and their errors, if any. (cherry picked from commit bf9fc126709af948c2a324ceb1b2696046c91cfe)
* clar: refactor explicitly run test behaviorEdward Thomson2018-10-261-11/+44
| | | | | | | | | | | | | | Previously, supplying `-s` to explicitly enable some test(s) would run the tests immediately from the argument parser. This forces us to set up the entire clar environment (for example: sandboxing) before argument parsing takes place. Refactor the behavior of `-s` to add the explicitly chosen tests to a list that is executed later. This untangles the argument parsing from the setup lifecycle, allowing us to use the arguments to perform the setup. (cherry picked from commit 90753a96515f85e2d0e79a16d3a06ba5b363c68e)
* buf tests: allocate a smaller size for the oomEdward Thomson2018-10-261-3/+15
| | | | | | | | | | | | | On Linux (where we run valgrind) allocate a smaller buffer, but still an insanely large size. This will cause malloc to fail but will not cause valgrind to report a likely error with a negative-sized malloc. Keep the original buffer size on non-Linux platforms: this is well-tested on them and changing it may be problematic. On macOS, for example, using the new size causes `malloc` to print a warning to stderr. (cherry picked from commit 219512e7989340d9efae8480fb79c08b91724014)
* clar: verify command line arguments before executeYoney2018-10-261-2/+9
| | | | | | | | | | | When executing `libgit2_clar -smerge -invalid_option`, it will first execute the merge test suite and afterwards output help because of the invalid option. With this changa, it verifies all options before execute. If there are any invalid options, it will output help and exit without actually executing the test suites. (cherry picked from commit 3275863134122892e2f8a8aa4ad0ce1c123a48ec)
* tests: online::clone: inline creds-test with nonexistent URLPatrick Steinhardt2018-10-191-4/+4
| | | | | | | | | | Right now, we test our credential callback code twice, once via SSH on localhost and once via a non-existent GitHub repository. While the first URL makes sense to be configurable, it does not make sense to hard-code the non-existing repository, which requires us to call tests multiple times. Instead, we can just inline the URL into another set of tests. (cherry picked from commit 54a1bf057a1123cf55ac3447c79761c817382f47)
* tests: online::clone: construct credential-URL from environmentPatrick Steinhardt2018-10-191-3/+13
| | | | | | | | | | | | | | | We support two types of passing credentials to the proxy, either via the URL or explicitly by specifying user and password. We test these types by modifying the proxy URL and executing the tests twice, which is in fact unnecessary and requires us to maintain the list of environment variables and test executions across multiple CI infrastructures. To fix the situation, we can just always pass the host, port, user and password to the tests. The tests can then assemble the complete URL either with or without included credentials, allowing us to test both cases in-process. (cherry picked from commit fea6092079d5c09b499e472efead2f7aa81ce8a1)
* tests: perf: build but exclude performance tests by defaultPatrick Steinhardt2018-10-191-13/+0
| | | | | | | | | | | | | | | | | | | | | Our performance tests (or to be more concrete, our single performance test) are not built by default, as they are always #ifdef'd out. While it is true that we don't want to run performance tests by default, not compiling them at all may cause code rot and is thus an unfavorable approach to handle this. We can easily improve this situation: this commit removes the #ifdef, causing the code to always be compiled. Furthermore, we add `-xperf` to the default command line parameters of `generate.py`, thus causing the tests to be excluded by default. Due to this approach, we are now able to execute the performance tests by passing `-sperf` to `libgit2_clar`. Unfortunately, we cannot execute the performance tests on Travis or AppVeyor as they rely on history being available for the libgit2 repository. As both do a shallow clone only, though, this is not given. (cherry picked from commit 543ec149b86a68e12dd141a6141e82850dabbf21)
* tests: iterator::workdir: fix reference count in stale testPatrick Steinhardt2018-10-191-1/+1
| | | | | | | | | | | The test `iterator::workdir::filesystem_gunk` is usually not executed, as it is guarded by the environment variable "GITTEST_INVASIVE_SPEED" due to its effects on speed. As such, it has become stale and does not account for new references which have meanwhile been added to the testrepo, causing it to fail. Fix this by raising the number of expected references to 15. (cherry picked from commit b8c14499f9940feaab08a23651a2ef24d27b17b7)
* tests: iterator_helpers: assert number of iterator itemsPatrick Steinhardt2018-10-191-2/+1
| | | | | | | | | | | When the function `expect_iterator_items` surpasses the number of expected items, we simply break the loop. This causes us to trigger an assert later on which has message attached, which is annoying when trying to locate the root error cause. Instead, directly assert that the current count is still smaller or equal to the expected count inside of the loop. (cherry picked from commit 9aba76364fcb4755930856a7bafc5294ed3ee944)
* tests: status::worktree: indicate skipped tests on Win32Patrick Steinhardt2018-10-194-0/+10
| | | | | | | | | | | Some function bodies of tests which are not applicable to the Win32 platform are completely #ifdef'd out instead of calling `cl_skip()`. This leaves us with no indication that these tests are not being executed at all and may thus cause decreased scrutiny when investigating skipped tests. Improve the situation by calling `cl_skip()` instead of just doing nothing. (cherry picked from commit 72c28ab011759dce113c2a0c7c36ebcd56bd6ddf)
* tests: online::clone: use URL of test serverPatrick Steinhardt2018-10-191-1/+1
| | | | | | | | | | | | All our tests running against a local SSH server usually read the server's URL from environment variables. But online::clone::ssh_cert test fails to do so and instead always connects to "ssh://localhost/foo". This assumption breaks whenever the SSH server is not running on the standard port, e.g. when it is running as a user. Fix the issue by using the URL provided by the environment. (cherry picked from commit c2c95ad0a210be4811c247be51664bfe8b2e830a)
* submodule: add failing test for option-injection protection in url and pathCarlos Martín Nieto2018-10-051-0/+80
|
* config_file: properly ignore includes without "path" valuePatrick Steinhardt2018-10-051-0/+15
| | | | | | | | | | | | | | | | In case a configuration includes a key "include.path=" without any value, the generated configuration entry will have its value set to `NULL`. This is unexpected by the logic handling includes, and as soon as we try to calculate the included path we will unconditionally dereference that `NULL` pointer and thus segfault. Fix the issue by returning early in both `parse_include` and `parse_conditional_include` in case where the `file` argument is `NULL`. Add a test to avoid future regression. The issue has been found by the oss-fuzz project, issue 10810. (cherry picked from commit d06d4220eec035466d1a837972a40546b8904330)
* tests: always unlink created config filesPatrick Steinhardt2018-10-051-2/+13
| | | | | | | | | | | | While our tests in config::include create a plethora of configuration files, most of them do not get removed at the end of each test. This can cause weird interactions with tests that are being run at a later stage if these later tests try to create files or directories with the same name as any of the created configuration files. Fix the issue by unlinking all created files at the end of these tests. (cherry picked from commit bf662f7cf8daff2357923446cf9d22f5d4b4a66b)
* smart_pkt: reorder and rename parameters of `git_pkt_parse_line`Patrick Steinhardt2018-10-031-12/+12
| | | | | | | | | 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 0b3dfbf425d689101663341beb94237614f1b5c2)
* smart_pkt: fix buffer overflow when parsing "ok" packetsPatrick Steinhardt2018-10-031-7/+4
| | | | | | | | | | | | | | | | | | | | | | | | | 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 a9f1ca09178af0640963e069a2142d5ced53f0b4)
* smart_pkt: fix buffer overflow when parsing "ACK" packetsPatrick Steinhardt2018-10-031-16/+11
| | | | | | | | | | | | | | 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 bc349045b1be8fb3af2b02d8554483869e54d5b8)
* tests: verify parsing logic for smart packetsPatrick Steinhardt2018-10-031-0/+348
| | | | | | | | | | | | | 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`. (cherry picked from commit 365d2720c1a5fc89f03fd85265c8b45195c7e4a8)
* util: introduce `git__prefixncmp` and consolidate implementationsEdward Thomson2018-10-011-0/+42
| | | | | | | | | | | | | 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. (cherry picked from commit 86219f40689c85ec4418575223f4376beffa45af)
* delta: fix out-of-bounds read of deltaPatrick Steinhardt2018-07-051-0/+9
| | | | | | | | | | | When computing the offset and length of the delta base, we repeatedly increment the `delta` pointer without checking whether we have advanced past its end already, which can thus result in an out-of-bounds read. Fix this by repeatedly checking whether we have reached the end. Add a test which would cause Valgrind to produce an error. Reported-by: Riccardo Schirone <rschiron@redhat.com> Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
* delta: fix sign-extension of big left-shiftPatrick Steinhardt2018-07-052-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our delta code was originally adapted from JGit, which itself adapted it from git itself. Due to this heritage, we inherited a bug from git.git in how we compute the delta offset, which was fixed upstream in 48fb7deb5 (Fix big left-shifts of unsigned char, 2009-06-17). As explained by Linus: Shifting 'unsigned char' or 'unsigned short' left can result in sign extension errors, since the C integer promotion rules means that the unsigned char/short will get implicitly promoted to a signed 'int' due to the shift (or due to other operations). This normally doesn't matter, but if you shift things up sufficiently, it will now set the sign bit in 'int', and a subsequent cast to a bigger type (eg 'long' or 'unsigned long') will now sign-extend the value despite the original expression being unsigned. One example of this would be something like unsigned long size; unsigned char c; size += c << 24; where despite all the variables being unsigned, 'c << 24' ends up being a signed entity, and will get sign-extended when then doing the addition in an 'unsigned long' type. Since git uses 'unsigned char' pointers extensively, we actually have this bug in a couple of places. In our delta code, we inherited such a bogus shift when computing the offset at which the delta base is to be found. Due to the sign extension we can end up with an offset where all the bits are set. This can allow an arbitrary memory read, as the addition in `base_len < off + len` can now overflow if `off` has all its bits set. Fix the issue by casting the result of `*delta++ << 24UL` to an unsigned integer again. Add a test with a crafted delta that would actually succeed with an out-of-bounds read in case where the cast wouldn't exist. Reported-by: Riccardo Schirone <rschiron@redhat.com> Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
* path: hand-code the zero-width joiner as UTF-8Carlos Martín Nieto2018-06-011-1/+5
|
* submodule: plug leaks from the escape detectionCarlos Martín Nieto2018-06-011-0/+2
|
* checkout: change symlinked .gitmodules file test to expect failureCarlos Martín Nieto2018-06-011-1/+6
| | | | | | When dealing with `core.proectNTFS` and `core.protectHFS` we do check against `.gitmodules` but we still have a failing test as the non-filesystem codepath does not check for it.
* path: reject .gitmodules as a symlinkCarlos Martín Nieto2018-06-012-197/+204
| | | | | | | | Any part of the library which asks the question can pass in the mode to have it checked against `.gitmodules` being a symlink. This is particularly relevant for adding entries to the index from the worktree and for checking out files.
* path: accept the name length as a parameterCarlos Martín Nieto2018-06-011-4/+4
| | | | | We may take in names from the middle of a string so we want the caller to let us know how long the path component is that we should be checking.
* checkout: add a failing test for refusing a symlinked .gitmodulesCarlos Martín Nieto2018-06-016-0/+8
| | | | | We want to reject these as they cause compatibility issues and can lead to git writing to files outside of the repository.
* path: hide the dotgit file functionsCarlos Martín Nieto2018-06-011-2/+0
| | | | | These can't go into the public API yet as we don't want to introduce API or ABI changes in a security release.
* path: add a function to detect an .gitmodules fileCarlos Martín Nieto2018-06-011-0/+110
| | | | | | | | Given a path component it knows what to pass to the filesystem-specific functions so we're protected even from trees which try to use the 8.3 naming rules to get around us matching on the filename exactly. The logic and test strings come from the equivalent git change.
* submodule: also validate Windows-separated paths for validityCarlos Martín Nieto2018-06-011-6/+37
| | | | | | | | Otherwise we would also admit `..\..\foo\bar` as a valid path and fail to protect Windows users. Ideally we would check for both separators without the need for the copied string, but this'll get us over the RCE.
* submodule: ignore submodules which include path traversal in their nameCarlos Martín Nieto2018-06-011-12/+17
| | | | | | | | | | | | | | If the we decide that the "name" of the submodule (i.e. its path inside `.git/modules/`) is trying to escape that directory or otherwise trick us, we ignore the configuration for that submodule. This leaves us with a half-configured submodule when looking it up by path, but it's the same result as if the configuration really were missing. The name check is potentially more strict than it needs to be, but it lets us re-use the check we're doing for the checkout. The function that encapsulates this logic is ready to be exported but we don't want to do that in a security release so it remains internal for now.
* submodule: add a failing test for a submodule escaping .git/modulesCarlos Martín Nieto2018-06-011-0/+60
| | | | We should pretend such submdules do not exist as it can lead to RCE.
* online tests: update auth for bitbucket testEdward Thomson2018-06-011-1/+1
| | | | | Update the settings to use a specific read-only token for accessing our test repositories in Bitbucket.
* online::clone: skip creds fallback testEdward Thomson2018-06-011-4/+34
| | | | | | | | | | | | At present, we have three online tests against bitbucket: one which specifies the credentials in the payload, one which specifies the correct credentials in the URL and a final one that specifies the incorrect credentials in the URL. Bitbucket has begun responding to the latter test with a 403, which causes us to fail. Break these three tests into separate tests so that we can skip the latter until this is resolved on Bitbucket's end or until we can change the test to a different provider.
View Lorry Controller Status