summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
* commit: fix reading out of bounds when parsing encodingPatrick Steinhardt2018-10-261-1/+1
| | | | | | | | | | | | | 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. (cherry picked from commit 7655b2d89e8275853d9921dd903dcdad9b3d4a7b)
* tag: fix out of bounds read when searching for tag messagePatrick Steinhardt2018-10-261-5/+5
| | | | | | | | | | | | | | | | | 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. (cherry picked from commit ee11d47e3d907b66eeff99e0ba1e1c71e05164b7)
* util: provide `git__memmem` functionPatrick Steinhardt2018-10-262-0/+44
| | | | | | | | | | | | | | | | | | | | | | | 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-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. (cherry picked from commit ea19efc19fa683d632af3e172868bc4350724813)
* util: avoid signed integer overflows in `git__strntol64`Patrick Steinhardt2018-10-261-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. (cherry picked from commit b09c1c7b636c4112e247adc24245c65f3f9478d0)
* util: remove `git__strtol32`Patrick Steinhardt2018-10-262-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. (cherry picked from commit 8d7fa88a9d5011b653035497b0f523e0f177b6a6)
* global: replace remaining use of `git__strtol32`Patrick Steinhardt2018-10-265-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. (cherry picked from commit 2613fbb26a3e1a34dda8a5d198c108626cfd6cc3)
* tree-cache: avoid out-of-bound reads when parsing treesPatrick Steinhardt2018-10-261-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. (cherry picked from commit 21652ee9de439e042cc2e69b208aa2ef8ce31147)
* util: remove unsafe `git__strtol64` functionPatrick Steinhardt2018-10-262-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. (cherry picked from commit 68deb2cc80ef19bf3a1915c26b5308b283a6d69a)
* config: remove last instance of `git__strntol64`Patrick Steinhardt2018-10-261-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. (cherry picked from commit 1a2efd10bde66f798375e2f47ba57ef00ad5c193)
* signature: avoid out-of-bounds reads when parsing signature datesPatrick Steinhardt2018-10-261-3/+5
| | | | | | | | | | | | 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. (cherry picked from commit 3db9aa6f79711103a331a2bbbd044a3c37d4f136)
* index: avoid out-of-bounds read when reading reuc entry stagePatrick Steinhardt2018-10-261-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. (cherry picked from commit 600ceadd1426b874ae0618651210a690a68b27e9)
* commit_list: avoid use of strtol64 without length limitPatrick Steinhardt2018-10-261-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. (cherry picked from commit 1a3fa1f5fafd433bdcf1834426d6963eff532125)
* smart subtransport: free url when resetting streamEdward Thomson2018-10-261-0/+5
| | | | | | Free the url field when resetting the stream to avoid leaking it. (cherry picked from commit ca2eb4608243162a13c427e74526b6422d5a6659)
* 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.
* config_file: properly ignore includes without "path" valuePatrick Steinhardt2018-10-051-1/+1
| | | | | | | | | | | | | | | | 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)
* 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. (cherry picked from commit 1bc5b05c614c7b10de021fa392943e8e6bd12c77)
* 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. (cherry picked from commit c05790a8a8dd4351e61fc06c0a06c6a6fb6134dc)
* 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. (cherry picked from commit 0b3dfbf425d689101663341beb94237614f1b5c2)
* 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. (cherry picked from commit 5fabaca801e1f5e7a1054be612e8fabec7cd6a7f)
* 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 ". (cherry picked from commit b5ba7af2d30c958b090dcf135749d9afe89ec703)
* 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) (cherry picked from commit a9f1ca09178af0640963e069a2142d5ced53f0b4)
* 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. (cherry picked from commit bc349045b1be8fb3af2b02d8554483869e54d5b8)
* 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. (cherry picked from commit 5edcf5d190f3b379740b223ff6a649d08fa49581)
* 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. (cherry picked from commit 786426ea6ec2a76ffe2515dc5182705fb3d44603)
* 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. (cherry picked from commit 40fd84cca68db24f325e460a40dabe805e7a5d35)
* 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. (cherry picked from commit 4a5804c983317100eed509537edc32d69c8d7aa2)
* util: introduce `git__prefixncmp` and consolidate implementationsEdward Thomson2018-10-012-16/+29
| | | | | | | | | | | | | 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)
* Verify ref_pkt's are long enoughNelson Elhage2018-10-011-0/+5
| | | | | | | | | | | If the remote sends a too-short packet, we'll allow `len` to go negative and eventually issue a malloc for <= 0 bytes on ``` pkt->head.name = git__malloc(alloclen); ``` (cherry picked from commit 437ee5a70711ac2e027877d71ee4ae17e5ec3d6c)
* smart: typedef git_pkt_type and clarify recv_pkt return typeEtienne Samson2018-10-012-30/+29
| | | | (cherry picked from commit 08961c9d0d6927bfcc725bd64c9a87dbcca0c52c)
* Small style tweak, and set an errorNelson Elhage2018-10-011-1/+11
| | | | (cherry picked from commit 895a668e19dc596e7b12ea27724ceb7b68556106)
* Remove GIT_PKT_PACK entirelyNelson Elhage2018-10-012-26/+3
| | | | (cherry picked from commit 90cf86070046fcffd5306915b57786da054d8964)
* Fix 'invalid packet line' for ng packets containing errorsChristian Schlack2018-10-011-7/+9
| | | | (cherry picked from commit 50dd7fea5ad1bf6c013b72ad0aa803a9c84cdede)
* Prevent heap-buffer-overflowbisho2018-10-011-1/+1
| | | | | | | | | | | | | | | | | | | | When running repack while doing repo writes, `packfile_load__cb()` can see some temporary files in the directory that are bigger than the usual, and makes `memcmp` overflow on the `p->pack_name` string. ASAN detected this. This just uses `strncmp`, that should not have any performance impact and is safe for comparing strings of different sizes. ``` ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200001a3f3 at pc 0x7f4a9e1976ec bp 0x7ffc1f80e100 sp 0x7ffc1f80d8b0 READ of size 89 at 0x61200001a3f3 thread T0 SCARINESS: 26 (multi-byte-read-heap-buffer-overflow) #0 0x7f4a9e1976eb in __interceptor_memcmp.part.78 (/build/cfgr-admin#link-tree/libtools_build_sanitizers_asan-ubsan-py.so+0xcf6eb) #1 0x7f4a518c5431 in packfile_load__cb /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:213 #2 0x7f4a518d9582 in git_path_direach /build/libgit2/0.27.0/src/libgit2-0.27.0/src/path.c:1134 #3 0x7f4a518c58ad in pack_backend__refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:347 #4 0x7f4a518c1b12 in git_odb_refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1511 #5 0x7f4a518bff5f in git_odb__freshen /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:752 #6 0x7f4a518c17d4 in git_odb_stream_finalize_write /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1415 #7 0x7f4a51b9d015 in Repository_write /build/pygit2/0.27.0/src/pygit2-0.27.0/src/repository.c:509 ``` (cherry picked from commit d22cd1f4a4c10ff47b04c57560e6765d77e5a8fd)
* config_parse: refactor error handling when parsing multiline variablesPatrick Steinhardt2018-10-011-19/+24
| | | | | | | | | | | The current error handling for the multiline variable parser is a bit fragile, as each error condition has its own code to clear memory. Instead, unify error handling as far as possible to avoid this repetitive code. While at it, make use of `GITERR_CHECK_ALLOC` to correctly handle OOM situations and verify that the buffer we print into does not run out of memory either. (cherry picked from commit bc63e1ef521ab5900dc0b0dcd578b8bf18627fb1)
* config: Fix a leak parsing multi-line config entriesNelson Elhage2018-10-011-0/+1
| | | | (cherry picked from commit 38b852558eb518f96c313cdcd9ce5a7af6ded194)
* config: convert unbounded recursion into a loopNelson Elhage2018-10-011-33/+29
| | | | (cherry picked from commit a03113e80332fba6c77f43b21d398caad50b4b89)
* smart_pkt: fix potential OOB-read when processing ng packetPatrick Steinhardt2018-08-061-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | OSS-fuzz has reported a potential out-of-bounds read when processing a "ng" smart packet: ==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6310000249c0 at pc 0x000000493a92 bp 0x7ffddc882cd0 sp 0x7ffddc882480 READ of size 65529 at 0x6310000249c0 thread T0 SCARINESS: 26 (multi-byte-read-heap-buffer-overflow) #0 0x493a91 in __interceptor_strchr.part.35 /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:673 #1 0x813960 in ng_pkt libgit2/src/transports/smart_pkt.c:320:14 #2 0x810f79 in git_pkt_parse_line libgit2/src/transports/smart_pkt.c:478:9 #3 0x82c3c9 in git_smart__store_refs libgit2/src/transports/smart_protocol.c:47:12 #4 0x6373a2 in git_smart__connect libgit2/src/transports/smart.c:251:15 #5 0x57688f in git_remote_connect libgit2/src/remote.c:708:15 #6 0x52e59b in LLVMFuzzerTestOneInput /src/download_refs_fuzzer.cc:145:9 #7 0x52ef3f in ExecuteFilesOnyByOne(int, char**) /src/libfuzzer/afl/afl_driver.cpp:301:5 #8 0x52f4ee in main /src/libfuzzer/afl/afl_driver.cpp:339:12 #9 0x7f6c910db82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291 #10 0x41d518 in _start When parsing an "ng" packet, we keep track of both the current position as well as the remaining length of the packet itself. But instead of taking care not to exceed the length, we pass the current pointer's position to `strchr`, which will search for a certain character until hitting NUL. It is thus possible to create a crafted packet which doesn't contain a NUL byte to trigger an out-of-bounds read. Fix the issue by instead using `memchr`, passing the remaining length as restriction. Furthermore, verify that we actually have enough bytes left to produce a match at all. OSS-Fuzz-Issue: 9406
* delta: fix overflow when computing limitPatrick Steinhardt2018-07-051-2/+4
| | | | | | | | | | | | | | When checking whether a delta base offset and length fit into the base we have in memory already, we can trigger an overflow which breaks the check. This would subsequently result in us reading memory from out of bounds of the base. The issue is easily fixed by checking for overflow when adding `off` and `len`, thus guaranteeting that we are never indexing beyond `base_len`. This corresponds to the git patch 8960844a7 (check patch_delta bounds more carefully, 2006-04-07), which adds these overflow checks. Reported-by: Riccardo Schirone <rschiron@redhat.com>
* delta: fix out-of-bounds read of deltaPatrick Steinhardt2018-07-051-8/+10
| | | | | | | | | | | 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-051-17/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* submodule: plug leaks from the escape detectionCarlos Martín Nieto2018-06-011-3/+10
|
* submodule: replace index with strchr which exists on WindowsCarlos Martín Nieto2018-06-011-1/+1
|
* submodule: the repostiory for _name_is_valid should not be constCarlos Martín Nieto2018-06-012-4/+3
| | | | | We might modify caches due to us trying to load the configuration to figure out what kinds of filesystem protections we should have.
* path: check for a symlinked .gitmodules in fs-agnostic codeCarlos Martín Nieto2018-06-011-8/+32
| | | | | We still compare case-insensitively to protect more thoroughly as we don't know what specifics we'll see on the system and it's the behaviour from git.
* path: reject .gitmodules as a symlinkCarlos Martín Nieto2018-06-017-18/+28
| | | | | | | | 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.
* index: stat before creating the entryCarlos Martín Nieto2018-06-011-7/+30
| | | | | This is so we have it available for the path validity checking. In a later commit we will start rejecting `.gitmodules` files as symlinks.
* path: accept the name length as a parameterCarlos Martín Nieto2018-06-012-36/+43
| | | | | 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.
* path: expose dotgit detection functions per filesystemCarlos Martín Nieto2018-06-012-3/+84
| | | | | These will be used by the checkout code to detect them for the particular filesystem they're on.
* path: hide the dotgit file functionsCarlos Martín Nieto2018-06-011-0/+21
| | | | | These can't go into the public API yet as we don't want to introduce API or ABI changes in a security release.