summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* version: bump to v0.27.8ethomson/v0.27.8Edward Thomson2019-01-251-2/+2
|
* CHANGELOG: update for v0.27.8Edward Thomson2019-01-251-0/+33
|
* ignore: remove now-useless check for LEADINGDIRPatrick Steinhardt2019-01-251-14/+3
| | | | | | | | | | | When checking whether a rule negates another rule, we were checking whether a rule had the `GIT_ATTR_FNMATCH_LEADINGDIR` flag set and, if so, added a "/*" to its end before passing it to `fnmatch`. Our code now sets `GIT_ATTR_FNMATCH_NOLEADINGDIR`, thus the `LEADINGDIR` flag shall never be set. Furthermore, due to the `NOLEADINGDIR` flag, trailing globs do not get consumed by our ignore parser anymore. Clean up code by just dropping this now useless logic.
* tests: status::ignore: fix style of a testPatrick Steinhardt2019-01-251-20/+15
|
* ignore: fix negative leading directory rules unignoring subdirectory filesPatrick Steinhardt2019-01-253-3/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When computing whether a file is ignored, we simply search for the first matching rule and return whether it is a positive ignore rule (the file is really ignored) or whether it is a negative ignore rule (the file is being unignored). Each rule has a set of flags which are being passed to `fnmatch`, depending on what kind of rule it is. E.g. in case it is a negative ignore we add a flag `GIT_ATTR_FNMATCH_NEGATIVE`, in case it contains a glob we set the `GIT_ATTR_FNMATCH_HASGLOB` flag. One of these flags is the `GIT_ATTR_FNMATCH_LEADINGDIR` flag, which is always set in case the pattern has a trailing "/*" or in case the pattern is negative. The flag causes the `fnmatch` function to return a match in case a string is a leading directory of another, e.g. "dir/" matches "dir/foo/bar.c". In case of negative patterns, this is wrong in certain cases. Take the following simple example of a gitignore: dir/ !dir/ The `LEADINGDIR` flag causes "!dir/" to match "dir/foo/bar.c", and we correctly unignore the directory. But take this example: *.test !dir/* We expect everything in "dir/" to be unignored, but e.g. a file in a subdirectory of dir should be ignored, as the "*" does not cross directory hierarchies. With `LEADINGDIR`, though, we would just see that "dir/" matches and return that the file is unignored, even if it is contained in a subdirectory. Instead, we want to ignore leading directories here and check "*.test". Afterwards, we have to iterate up to the parent directory and do the same checks. To fix the issue, disallow matching against leading directories in gitignore files. This can be trivially done by just adding the `GIT_ATTR_FNMATCH_NOLEADINGDIR` to the spec passed to `git_attr_fnmatch__parse`. Due to a bug in that function, though, this flag is being ignored for negative patterns, which is fixed in this commit, as well. As a last fix, we need to ignore rules that are supposed to match a directory when our path itself is a file. All together, these changes fix the described error case.
* patch_parse: remove unused function `parse_number`Patrick Steinhardt2019-01-251-20/+0
| | | | | | | The function `parse_number` was replaced by `git_parse_advance_digit` which is provided by the parser interface in commit 252f2eeee (parse: implement and use `git_parse_advance_digit`, 2017-07-14). As there are no remaining callers, remove it.
* strntol: fix out-of-bounds reads when parsing numbers with leading signPatrick Steinhardt2019-01-252-2/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* tree: fix integer overflow when reading unreasonably large filemodesPatrick Steinhardt2019-01-251-13/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | The `parse_mode` option uses an open-coded octal number parser. The parser is quite naive in that it simply parses until hitting a character that is not in the accepted range of '0' - '7', completely ignoring the fact that we can at most accept a 16 bit unsigned integer as filemode. If the filemode is bigger than UINT16_MAX, it will thus overflow and provide an invalid filemode for the object entry. Fix the issue by using `git__strntol32` instead and doing a bounds check. As this function already handles overflows, it neatly solves the problem. Note that previously, `parse_mode` was also skipping the character immediately after the filemode. In proper trees, this should be a simple space, but in fact the parser accepted any character and simply skipped over it. As a consequence of using `git__strntol32`, we now need to an explicit check for a trailing whitespace after having parsed the filemode. Because of the newly introduced error message, the test object::tree::parse::mode_doesnt_cause_oob_read needs adjustment to its error message check, which in fact is a good thing as it demonstrates that we now fail looking for the whitespace immediately following the filemode. Add a test that shows that we will fail to parse such invalid filemodes now.
* strntol: fix detection and skipping of base prefixesPatrick Steinhardt2019-01-252-11/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* tree: fix mode parsing reading out-of-boundsPatrick Steinhardt2019-01-251-3/+4
| | | | | | | | | | | 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.
* strntol: fix out-of-bounds read when skipping leading spacesPatrick Steinhardt2019-01-252-2/+15
| | | | | | | | | | | | | | 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.
* signature: fix out-of-bounds read when parsing timezone offsetPatrick Steinhardt2019-01-182-1/+21
| | | | | | | | | | | | | | | | | | | | | | | | | When parsing a signature's timezone offset, we first check whether there is a timezone at all by verifying that there are still bytes left to read following the time itself. The check thus looks like `time_end + 1 < buffer_end`, which is actually correct in this case. After setting the timezone's start pointer to that location, we compute the remaining bytes by using the formula `buffer_end - tz_start + 1`, re-using the previous `time_end + 1`. But this is in fact missing the braces around `(tz_start + 1)`, thus leading to an overestimation of the remaining bytes by a length of two. In case of a non-NUL terminated buffer, this will result in an overflow. The function `git_signature__parse` is only used in two locations. First is `git_signature_from_buffer`, which only accepts a string without a length. The string thus necessarily has to be NUL terminated and cannot trigger the issue. The other function is `git_commit__parse_raw`, which can in fact trigger the error as it may receive non-NUL terminated commit data. But as objects read from the ODB are always NUL-terminated by us as a cautionary measure, it cannot trigger the issue either. In other words, this error does not have any impact on security.
* tests: address two null argument instancesNoah Pendleton2019-01-182-2/+4
| | | | | | | | | | | | | | | | | | | | | | Handle two null argument cases that occur in the unit tests. One is in library code, the other is in test code. Detected by running unit tests with undefined behavior sanitizer: ```bash # build mkdir build && cd build cmake -DBUILD_CLAR=ON -DCMAKE_C_FLAGS="-fsanitize=address \ -fsanitize=undefined -fstack-usage -static-libasan" .. cmake --build . # run with asan ASAN_OPTIONS="allocator_may_return_null=1" ./libgit2_clar ... ............../libgit2/src/apply.c:316:3: runtime error: null pointer \ passed as argument 1, which is declared to never be null ...................../libgit2/tests/apply/fromfile.c:46:3: runtime \ error: null pointer passed as argument 1, which is declared to never be null ```
* commit: fix out-of-bound reads when parsing truncated author fieldsPatrick Steinhardt2019-01-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | While commit objects usually should have only one author field, our commit parser actually handles the case where a commit has multiple author fields because some tools that exist in the wild actually write them. Detection of those additional author fields is done by using a simple `git__prefixcmp`, checking whether the current line starts with the string "author ". In case where we are handed a non-NUL-terminated string that ends directly after the space, though, we may have an out-of-bounds read of one byte when trying to compare the expected final NUL byte. Fix the issue by using `git__prefixncmp` instead of `git_prefixcmp`. Unfortunately, a test cannot be easily written to catch this case. While we could test the last error message and verify that it didn't in fact fail parsing a signature (because that would indicate that it has in fact tried to parse the additional "author " field, which it shouldn't be able to detect in the first place), this doesn't work as the next line needs to be the "committer" field, which would error out with the same error message even if we hadn't done an out-of-bounds read. As objects read from the object database are always NUL terminated, this issue cannot be triggered in normal code and thus it's not security critical.
* config: fix adding files if their parent directory is a filePatrick Steinhardt2019-01-182-1/+21
| | | | | | | | | | | | | When we try to add a configuration file with `git_config_add_file_ondisk`, we treat nonexisting files as empty. We do this by performing a stat call, ignoring ENOENT errors. This works just fine in case the file or any of its parents simply does not exist, but there is also the case where any of the parent directories is not a directory, but a file. So e.g. trying to add a configuration file "/dev/null/.gitconfig" will fail, as `errno` will be ENOTDIR instead of ENOENT. Catch ENOTDIR in addition to ENOENT to fix the issue. Add a test that verifies we are able to add configuration files with such an invalid path file just fine.
* Typesetting conventionsJoe Rabinoff2019-01-181-9/+9
|
* Removed one null checkJoe Rabinoff2019-01-181-3/+2
|
* Fix segfault in loose_backend__readstreamJoe Rabinoff2019-01-181-5/+10
| | | | | If the routine exits with error before stream or hash_ctx is initialized, the program will segfault when trying to free them.
* make proxy_stream_close close target stream even on errorsAnders Borum2019-01-181-0/+6
| | | | | | | | When git_filter_apply_fn callback returns a error while smudging proxy_stream_close ends up returning without closing the stream. This is turn makes blob_content_to_file crash as it asserts the stream being closed whether there are errors or not. Closing the target stream on error fixes this problem.
* annotated_commit: peel to commit instead of assuming we have oneCarlos Martín Nieto2019-01-181-4/+4
| | | | | | We want to allow the creation of annotated commits out of annotated tags and for that we have to peel the reference all the way to the commit instead of stopping at the first id it provides.
* refs: constify git_reference_peelCarlos Martín Nieto2019-01-182-8/+9
| | | | | | | We have no need to take a non-const reference. This does involve some other work to make sure we don't mix const and non-const variables, but by splitting what we want each variable to do we can also simplify the logic for when we do want to free a new reference we might have allocated.
* annotated_commit: add failing test for looking up from annotated tagCarlos Martín Nieto2019-01-181-0/+26
|
* config: assert that our parameters are validEtienne Samson2019-01-181-0/+2
| | | CID 1395011
* refs: assert that we're passed valid refs when renamingEtienne Samson2019-01-181-0/+2
| | | CID 1382962
* diff: assert that we're passed a valid git_diff objectEtienne Samson2019-01-181-0/+2
| | | CID 1386176, 1386177, 1388219
* submodule: grab the error while loading from configEtienne Samson2019-01-181-1/+1
| | | | | | | | Previously, an error in `git_config_next` would be mistaken as a successful load, because the previous call would have succeeded. Coverity saw the subsequent check for a completed iteration as dead, so let's make it useful again. CID 1391374
* Merge pull request #4916 from libgit2/ethomson/backport_0278Edward Thomson2018-12-191-5/+4
|\ | | | | smart transport: only clear url on hard reset
| * smart transport: only clear url on hard resetethomson/backport_0278Edward Thomson2018-12-191-5/+4
|/ | | | | | | | | | | | | | | | | | | | | | After creating a transport for a server, we expect to be able to call `connect`, then invoke subsequent `action` calls. We provide the URL to these `action` calls, although our built-in transports happen to ignore it since they've already parsed it into an internal format that they intend to use (`gitno_connection_data`). In ca2eb4608243162a13c427e74526b6422d5a6659, we began clearing the URL field after a connection, meaning that subsequent calls to transport `action` callbacks would get a NULL URL, which went undetected since the builtin transports ignore the URL when they're already connected (instead of re-parsing it into an internal format). Downstream custom transport implementations (eg, LibGit2Sharp) did notice this change, however. Since `reset_stream` is called even when we're not closing the subtransport, update to only clear the URL when we're closing the subtransport. This ensures that `action` calls will get the correct URL information even after a connection.
* Merge pull request #4846 from pks-t/pks/v0.27.6v0.27.7Patrick Steinhardt2018-10-2629-66/+457
|\ | | | | Bugix release v0.27.7
| * version: bump to v0.27.7Patrick Steinhardt2018-10-261-2/+2
| |
| * CHANGELOG: update for v0.27.7Patrick Steinhardt2018-10-261-0/+45
| |
| * winhttp: retry erroneously failing requestsEdward Thomson2018-10-261-14/+18
| | | | | | | | | | | | | | | | | | | | | | | | Early Windows TLS 1.2 implementations have an issue during key exchange with OpenSSL implementations that cause negotiation to fail with the error "the buffer supplied to a function was too small." This is a transient error on the connection, so when that error is received, retry up to 5 times to create a connection to the remote server before actually giving up. (cherry picked from commit dc371e3c5903760cc2334a0acfac9bce04479773)
| * git_remote_prune to be O(n * logn)Marcin Krystianc2018-10-261-1/+1
| | | | | | | | (cherry picked from commit bfec6526e931d7f6ac5ecc38c37e76163092bfda)
| * ignore unsupported http authentication schemesAnders Borum2018-10-261-1/+4
| | | | | | | | | | | | | | | | | | | | | | auth_context_match returns 0 instead of -1 for unknown schemes to not fail in situations where some authentication schemes are supported and others are not. apply_credentials is adjusted to handle auth_context_match returning 0 without producing authentication context. (cherry picked from commit 475db39bb4c44a2221f340c66c227f555e478d10)
| * transport/http: do not return success if we failed to get a schemeEtienne Samson2018-10-261-1/+1
| | | | | | | | | | | | | | Otherwise we return a NULL context, which will get dereferenced in apply_credentials. (cherry picked from commit 1c949ce1483ca22a29e8f523360999cbbe411a50)
| * cmake: explicitly enable int-conversion warningsPatrick Steinhardt2018-10-261-0/+1
| | | | | | | | | | | | | | | | | | | | While GCC enables int-conversion warnings by default, it will currently only warn about such errors even in case where "-DENABLE_WERROR=ON" has been passed to CMake. Explicitly enable int-conversion warnings by using our `ENABLE_WARNINGS` macro, which will automatically use "-Werror=int-conversions" in case it has been requested by the user. (cherry picked from commit aa0ae59a2a31dc0ee5cc987066903d135a5f9e79)
| * tests: fix warning for implicit conversion of integer to pointerPatrick Steinhardt2018-10-261-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | GCC warns by default when implicitly converting integers to pointers or the other way round, and commit fa48d2ea7 (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. (cherry picked from commit dbb4a5866fcbb121000a705e074f679445d6916b)
| * cmake: enable -Wformat and -Wformat-securityCarlos Martín Nieto2018-10-261-0/+2
| | | | | | | | | | | | | | We do not currently have any warnings in this regard, but it's good practice to have them on in case we introduce something. (cherry picked from commit f2c1153d4fa09a36be7c6b87e4f55325f6e5031e)
| * fix check if blob is uninteresting when inserting tree to packbuilderAnders Borum2018-10-261-1/+1
| | | | | | | | | | | | | | | | | | | | Blobs that have been marked as uninteresting should not be inserted into packbuilder when inserting a tree. The check as to whether a blob was uninteresting looked at the status for the tree itself instead of the blob. This could cause significantly larger packfiles. (cherry picked from commit b36cc7a4013a47856dade4226edc657906b82431)
| * revwalk: only check the first commit in the list for an earlier timestampCarlos Martín Nieto2018-10-261-2/+8
| | | | | | | | | | | | | | | | | | This is not a big deal, but it does make us match git more closely by checking only the first. The lists are sorted already, so there should be no functional difference other than removing a possible check from every iteration in the loop. (cherry picked from commit 12a1790d8e71087056d2b2de936ddae439e1d94c)
| * revwalk: use the max value for a signed integerCarlos Martín Nieto2018-10-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When porting, we overlooked that the difference between git's and our's time representation and copied their way of getting the max value. Unfortunately git was using unsigned integers, so `~0ll` does correspond to their max value, whereas for us it corresponds to `-1`. This means that we always consider the last date to be smaller than the current commit's and always think commits are interesting. Change the initial value to the macro that gives us the maximum value on each platform so we can accurately consider commits interesting or not. (cherry picked from commit 46f35127b6fcfab87cb80d1b772ac7c662eafd38)
| * vector: do not realloc 0-size vectorsEtienne Samson2018-10-261-0/+3
| | | | | | | | (cherry picked from commit e0afd1c21c4421cec4f67162021f835e2bbb7df6)
| * vector: do not malloc 0-length vectors on dupEtienne Samson2018-10-262-8/+29
| | | | | | | | (cherry picked from commit fa48d2ea7d2d5dc9620e5c9f05ba8d788775582b)
| * index: release the snapshot instead of freeing the indexEtienne Samson2018-10-261-1/+1
| | | | | | | | | | | | | | | | 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 c70713d6e4af563696563e410864eb4a6507757d)
| * cmake: detect and use libc-provided iconvPatrick Steinhardt2018-10-261-6/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 2e2d8c6493ec4d151c55d7421c93126267ee8e6d)
| * remote: set the error before cleanupEtienne Samson2018-10-261-2/+2
| | | | | | | | | | | | Otherwise we'll return stack data to the caller. (cherry picked from commit 22d013b657c5957fde31641351cb72d08cc192ae)
| * revwalk: The left operand of '<' is a garbage valueEtienne Samson2018-10-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | 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 aa8cb5866f1eabd92c8c08f7a8610d42df07375f)
| * worktree: unlock should return 1 when the worktree isn't lockedEtienne Samson2018-10-262-3/+3
| | | | | | | | | | | | | | | | 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 59c2e70eeee8b2bae79d05060599114a5f6d737a)
| * Fix leak in index.cabyss72018-10-261-1/+2
| | | | | | | | (cherry picked from commit 581d5492f6afdaf31a10e51187466a80ffc9f76f)
| * util: make the qsort_r check work on macOSEtienne Samson2018-10-265-7/+142
| | | | | | | | | | | | | | | | | | | | 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 1a9cc18260b68b149476adb6f39e37ab47d3d21f)
View Lorry Controller Status