summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
...
| * | | | | util: fix out of bounds read in error messagePatrick Steinhardt2018-10-191-3/+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.
| * | | | | util: avoid signed integer overflows in `git__strntol64`Patrick Steinhardt2018-10-181-3/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | util: remove `git__strtol32`Patrick Steinhardt2018-10-182-7/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | global: replace remaining use of `git__strtol32`Patrick Steinhardt2018-10-185-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | tree-cache: avoid out-of-bound reads when parsing treesPatrick Steinhardt2018-10-181-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | util: remove unsafe `git__strtol64` functionPatrick Steinhardt2018-10-182-7/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | config: remove last instance of `git__strntol64`Patrick Steinhardt2018-10-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | signature: avoid out-of-bounds reads when parsing signature datesPatrick Steinhardt2018-10-181-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | index: avoid out-of-bounds read when reading reuc entry stagePatrick Steinhardt2018-10-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | | commit_list: avoid use of strtol64 without length limitPatrick Steinhardt2018-10-181-1/+3
| |/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | | util: allow callers to reset custom allocatorsEdward Thomson2018-10-211-5/+13
| | | | | | | | | | | | | | | | | | | | | | | | | Provide a utility to reset custom allocators back to their default. This is particularly useful for testing.
* | | | | Merge pull request #4852 from libgit2/ethomson/unc_pathsEdward Thomson2018-10-205-96/+122
|\ \ \ \ \ | | | | | | | | | | | | Win32 path canonicalization refactoring
| * | | | | win32: refactor `git_win32_path_remove_namespace`ethomson/unc_pathsEdward Thomson2018-10-191-25/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update `git_win32_path_remove_namespace` to disambiguate the prefix being removed versus the prefix being added. Now we remove the "namespace", and (may) add a "prefix" in its place. Eg, we remove the `\\?\` namespace. We remove the `\\?\UNC\` namespace, and replace it with the `\\` prefix. This aids readability somewhat. Additionally, use pointer arithmetic instead of offsets, which seems to also help readability.
| * | | | | win32: rename `git_win32__canonicalize_path`Edward Thomson2018-10-195-108/+111
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The internal API `git_win32__canonicalize_path` is far, far too easily confused with the internal API `git_win32_path_canonicalize`. The former removes the namespace prefix from a path (eg, given `\\?\C:\Temp\foo`, it returns `C:\Temp\foo`, and given `\\?\UNC\server\share`, it returns `\\server\share`). As such, rename it to `git_win32_path_remove_namespace`. `git_win32_path_canonicalize` remains unchanged.
| * | | | | Fix issue with path canonicalization for Win32 pathsGabriel DeBacker2018-09-301-2/+14
| | | | | |
* | | | | | Merge pull request #4840 from libgit2/cmn/validity-tree-from-unowned-indexEdward Thomson2018-10-201-29/+34
|\ \ \ \ \ \ | | | | | | | | | | | | | | Check object existence when creating a tree from an index
| * | | | | | tree: unify the entry validity checksCarlos Martín Nieto2018-10-081-29/+34
| | |_|_|_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have two similar functions, `git_treebuilder_insert` and `append_entry` which are used in different codepaths as part of creating a new tree. The former learnt to check for object existence under strict object creation, but the latter did not. This allowed the creation of a tree from an unowned index to bypass some of the checks and create a tree pointing to a nonexistent object. Extract a single function which performs these checks and call it from both codepaths. In `append_entry` we still do not validate when asked not to, as this is data which is already in the tree and we want to allow users to deal with repositories which already have some invalid data.
* | | | | | Merge branch 'issue-4203'Edward Thomson2018-10-201-1/+6
|\ \ \ \ \ \ | |_|_|_|/ / |/| | | | |
| * | | | | merge: don't leak the index during reloadsethomson/issue-4203Edward Thomson2018-10-201-3/+4
| | | | | |
| * | | | | merge: add error handling for index reloadEtiene Dalcol2017-11-111-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | Cleans up should git_repository_index or git_index_read fail
| * | | | | merge: reload index before git_mergeGreg Collinge2017-11-111-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the index in memory is different from the index on the disk, previously merge would abort with GIT_ECONFLICT. Reload the index before merging to fix this. Fixes #4203
* | | | | | Merge pull request #4819 from libgit2/cmn/config-nonewlinePatrick Steinhardt2018-10-191-6/+26
|\ \ \ \ \ \ | |_|_|_|/ / |/| | | | | Configuration variables can appear on the same line as the section header
| * | | | | config: variables might appear on the same line as a section headercmn/config-nonewlineCarlos Martín Nieto2018-10-151-6/+26
| | |_|_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While rare and a machine would typically not generate such a configuration file, it is nevertheless valid to write [foo "bar"] baz = true and we need to deal with that instead of assuming everything is on its own line.
* | | | | Merge pull request #4849 from libgit2/cmn/expose-gitfile-checkEdward Thomson2018-10-171-38/+2
|\ \ \ \ \ | | | | | | | | | | | | path: export the dotgit-checking functions
| * | | | | path: export the dotgit-checking functionscmn/expose-gitfile-checkCarlos Martín Nieto2018-10-151-38/+2
| |/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These checks are preformed by libgit2 on checkout, but they're also useful for performing checks in applications which do not involve checkout. Expose them under `sys/` as it's still fairly in the weeds even for this library.
* | | | | Merge pull request #4845 from pks-t/pks/object-fuzzerCarlos Martín Nieto2018-10-151-1/+3
|\ \ \ \ \ | | | | | | | | | | | | Object parsing fuzzer
| * | | | | object: properly propagate errors on parsing failuresPatrick Steinhardt2018-10-111-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When failing to parse a raw object fromits data, we free the partially parsed object but then fail to propagate the error to the caller. This may lead callers to operate on objects with invalid memory, which will sooner or later cause the program to segfault. Fix the issue by passing up the error code returned by `parse_raw`.
* | | | | | Merge commit 'afd10f0' (Follow 308 redirects)Carlos Martín Nieto2018-10-151-1/+2
|\ \ \ \ \ \ | |_|/ / / / |/| | | | |
| * | | | | Follow 308 redirects (as used by GitLab)Zander Brown2018-10-131-1/+2
| |/ / / /
* | | | | Merge pull request #4842 from nelhage/fuzz-config-memoryPatrick Steinhardt2018-10-122-3/+4
|\ \ \ \ \ | |_|_|/ / |/| | | | config: Port config_file_fuzzer to the new in-memory backend.
| * | | | Apply code review feedbackNelson Elhage2018-10-111-1/+1
| | | | |
| * | | | config: Refactor `git_config_backend_from_string` to take a lengthNelson Elhage2018-10-092-3/+4
| | | | |
* | | | | Merge pull request #4830 from pks-t/pks/diff-stats-rename-commonEdward Thomson2018-10-071-4/+18
|\ \ \ \ \ | | | | | | | | | | | | diff_stats: use git's formatting of renames with common directories
| * | | | | diff_stats: use git's formatting of renames with common directoriesPatrick Steinhardt2018-10-041-4/+18
| | |_|_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In cases where a file gets renamed such that the directories containing it previous and after the rename have a common prefix, then git will avoid printing this prefix twice and instead format the rename as "prefix/{old => new}". We currently didn't do anything like that, but simply printed "prefix/old -> prefix/new". Adjust our behaviour to instead match upstream. Adjust the test for this behaviour to expect the new format.
* | | | | ignore unsupported http authentication schemesAnders Borum2018-10-061-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.
* | | | Merge pull request #4837 from pks-t/cmn/reject-option-submodule-url-pathPatrick Steinhardt2018-10-051-8/+23
|\ \ \ \ | | | | | | | | | | submodule: ignore path and url attributes if they look like options
| * | | | submodule: ignore path and url attributes if they look like optionsCarlos Martín Nieto2018-10-051-8/+23
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These can be used to inject options in an implementation which performs a recursive clone by executing an external command via crafted url and path attributes such that it triggers a local executable to be run. The library is not vulnerable as we do not rely on external executables but a user of the library might be relying on that so we add this protection. This matches this aspect of git's fix for CVE-2018-17456.
* | | | Merge pull request #4836 from pks-t/pks/smart-packetsPatrick Steinhardt2018-10-053-109/+125
|\ \ \ \ | | | | | | | | | | Smart packet security fixes
| * | | | smart_pkt: do not accept callers passing in no line lengthPatrick Steinhardt2018-10-031-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Right now, we simply ignore the `linelen` parameter of `git_pkt_parse_line` in case the caller passed in zero. But in fact, we never want to assume anything about the provided buffer length and always want the caller to pass in the available number of bytes. And in fact, checking all the callers, one can see that the funciton is never being called in case where the buffer length is zero, and thus we are safe to remove this check.
| * | | | smart_pkt: return parsed length via out-parameterPatrick Steinhardt2018-10-031-29/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `parse_len` function currently directly returns the parsed length of a packet line or an error code in case there was an error. Instead, convert this to our usual style of using the return value as error code only and returning the actual value via an out-parameter. Thus, we can now convert the output parameter to an unsigned type, as the size of a packet cannot ever be negative. While at it, we also move the check whether the input buffer is long enough into `parse_len` itself. We don't really want to pass around potentially non-NUL-terminated buffers to functions without also passing along the length, as this is dangerous in the unlikely case where other callers for that function get added. Note that we need to make sure though to not mess with `GIT_EBUFS` error codes, as these indicate not an error to the caller but that he needs to fetch more data.
| * | | | smart_pkt: reorder and rename parameters of `git_pkt_parse_line`Patrick Steinhardt2018-10-033-24/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | smart_pkt: fix buffer overflow when parsing "unpack" packetsPatrick Steinhardt2018-10-031-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When checking whether an "unpack" packet returned the "ok" status or not, we use a call to `git__prefixcmp`. In case where the passed line isn't properly NUL terminated, though, this may overrun the line buffer. Fix this by using `git__prefixncmp` instead.
| * | | | smart_pkt: fix "ng" parser accepting non-space characterPatrick Steinhardt2018-10-031-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When parsing "ng" packets, we blindly assume that the character immediately following the "ng" prefix is a space and skip it. As the calling function doesn't make sure that this is the case, we can thus end up blindly accepting an invalid packet line. Fix the issue by using `git__prefixncmp`, checking whether the line starts with "ng ".
| * | | | smart_pkt: fix buffer overflow when parsing "ok" packetsPatrick Steinhardt2018-10-031-9/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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)
| * | | | smart_pkt: fix buffer overflow when parsing "ACK" packetsPatrick Steinhardt2018-10-031-14/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | smart_pkt: adjust style of "ref" packet parsing functionPatrick Steinhardt2018-10-031-25/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While the function parsing ref packets doesn't have any immediately obvious buffer overflows, it's style is different to all the other parsing functions. Instead of checking buffer length while we go, it does a check up-front. This causes the code to seem a lot more magical than it really is due to some magic constants. Refactor the function to instead make use of the style of other packet parser and verify buffer lengths as we go.
| * | | | smart_pkt: check whether error packets are prefixed with "ERR "Patrick Steinhardt2018-10-031-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the `git_pkt_parse_line` function, we determine what kind of packet a given packet line contains by simply checking for the prefix of that line. Except for "ERR" packets, we always only check for the immediate identifier without the trailing space (e.g. we check for an "ACK" prefix, not for "ACK "). But for "ERR" packets, we do in fact include the trailing space in our check. This is not really much of a problem at all, but it is inconsistent with all the other packet types and thus causes confusion when the `err_pkt` function just immediately skips the space without checking whether it overflows the line buffer. Adjust the check in `git_pkt_parse_line` to not include the trailing space and instead move it into `err_pkt` for consistency.
| * | | | smart_pkt: explicitly avoid integer overflows when parsing packetsPatrick Steinhardt2018-10-032-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When parsing data, progress or error packets, we need to copy the contents of the rest of the current packet line into the flex-array of the parsed packet. To keep track of this array's length, we then assign the remaining length of the packet line to the structure. We do have a mismatch of types here, as the structure's `len` field is a signed integer, while the length that we are assigning has type `size_t`. On nearly all platforms, this shouldn't pose any problems at all. The line length can at most be 16^4, as the line's length is being encoded by exactly four hex digits. But on a platforms with 16 bit integers, this assignment could cause an overflow. While such platforms will probably only exist in the embedded ecosystem, we still want to avoid this potential overflow. Thus, we now simply change the structure's `len` member to be of type `size_t` to avoid any integer promotion.
| * | | | smart_pkt: honor line length when determining packet typePatrick Steinhardt2018-10-031-6/+6
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | When we parse the packet type of an incoming packet line, we do not verify that we don't overflow the provided line buffer. Fix this by using `git__prefixncmp` instead and passing in `len`. As we have previously already verified that `len <= linelen`, we thus won't ever overflow the provided buffer length.
* | | | config_file: properly ignore includes without "path" valuePatrick Steinhardt2018-10-051-1/+4
| |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
View Lorry Controller Status