summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
...
| * delta: fix sign-extension of big left-shiftPatrick Steinhardt2018-07-053-17/+28
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Merge pull request #4666 from pks-t/pks/v0.26.4v0.26.4Edward Thomson2018-06-0420-246/+841
|\ | | | | Release v0.26.4
| * version: bump library version to 0.26.4Patrick Steinhardt2018-06-011-2/+2
| |
| * CHANGELOG: update for v0.26.4Patrick Steinhardt2018-06-011-0/+20
| |
| * 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-012-3/+12
| |
| * 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.
| * 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-019-215/+232
| | | | | | | | | | | | | | | | 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-013-40/+47
| | | | | | | | | | 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: 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-013-39/+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.
| * path: add functions to detect .gitconfig and .gitattributesCarlos Martín Nieto2018-06-012-0/+47
| |
| * path: add a function to detect an .gitmodules fileCarlos Martín Nieto2018-06-012-0/+123
| | | | | | | | | | | | | | | | 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.
| * path: provide a generic function for checking dogit files on NTFSCarlos Martín Nieto2018-06-011-0/+53
| | | | | | | | | | It checks against the 8.3 shortname variants, including the one which includes the checksum as part of its name.
| * path: provide a generic dogit checking function for HFSCarlos Martín Nieto2018-06-011-6/+19
| | | | | | | | This lets us check for other kinds of reserved files.
| * submodule: also validate Windows-separated paths for validityCarlos Martín Nieto2018-06-012-15/+65
| | | | | | | | | | | | | | | | 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-013-15/+55
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Merge pull request #4475 from pks-t/pks/v0.26.1-backportsv0.26.3Patrick Steinhardt2018-03-12152-153/+1026
|\ | | | | v0.26.3 backports
| * Bump version to v0.26.3Patrick Steinhardt2018-03-121-2/+2
| |
| * CHANGELOG.md: update for v0.26.3Patrick Steinhardt2018-03-121-0/+25
| |
| * curl: explicitly initialize and cleanup global curl statePatrick Steinhardt2018-03-103-2/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | Our curl-based streams make use of the easy curl interface. This interface automatically initializes and de-initializes the global curl state by calling out to `curl_global_init` and `curl_global_cleanup`. Thus, all global state will be repeatedly re-initialized when creating multiple curl streams in succession. Despite being inefficient, this is not thread-safe due to `curl_global_init` being not thread-safe itself. Thus a multi-threaded programing handling multiple curl streams at the same time is inherently racy. Fix the issue by globally initializing and cleaning up curl's state.
| * tree: initialize the id we use for testing submodule insertionsCarlos Martín Nieto2018-03-101-0/+1
| | | | | | | | | | | | Instead of laving it uninitialized and relying on luck for it to be non-zero, let's give it a dummy hash so we make valgrind happy (in this case the hash comes from `sha1sum </dev/null`.
| * win32: strncmp -> git__strncmpEdward Thomson2018-03-101-1/+1
| | | | | | | | | | | | | | | | The win32 C library is compiled cdecl, however when configured with `STDCALL=ON`, our functions (and function pointers) will use the stdcall calling convention. You cannot set a `__stdcall` function pointer to a `__cdecl` function, so it's easier to just use our `git__strncmp` instead of sorting that mess out.
| * winhttp: enable TLS 1.2 on Windows 7 and earlierEdward Thomson2018-03-101-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | Versions of Windows prior to Windows 8 do not enable TLS 1.2 by default, though support may exist. Try to enable TLS 1.2 support explicitly on connections. This request may fail if the operating system does not have TLS 1.2 support - the initial release of Vista lacks TLS 1.2 support (though it is available as a software update) and XP completely lacks TLS 1.2 support. If this request does fail, the HTTP context is still valid, and still maintains the original protocol support. So we ignore the failure from this operation.
| * winhttp: include constants for TLS 1.1/1.2 supportEdward Thomson2018-03-101-5/+8
| | | | | | | | | | For platforms that do not define `WINHTTP_FLAG_SECURE_PROTOCOL_TLS1_1` and/or `WINHTTP_FLAG_SECURE_PROTOCOL_TLS1_2`.
| * mingw: update TLS option flagsEdward Thomson2018-03-102-4/+11
| | | | | | | | | | | | | | | | Include the constants for `WINHTTP_FLAG_SECURE_PROTOCOL_TLS1_1` and `WINHTTP_FLAG_SECURE_PROTOCOL_TLS1_2` so that they can be used by mingw. This updates both the `deps/winhttp` framework (for classic mingw) and adds the defines for mingw64, which does not use that framework.
| * checkout test: further ensure workdir perms are updatedEdward Thomson2018-03-101-0/+25
| | | | | | | | | | | | | | When both the index _and_ the working directory has changed permissions on a file permissions on a file - but only the permissions, such that the contents of the file are identical - ensure that `git_checkout` updates the permissions to match the checkout target.
| * checkout test: ensure workdir perms are updatedEdward Thomson2018-03-101-0/+20
| | | | | | | | | | | | | | When the working directory has changed permissions on a file - but only the permissions, such that the contents of the file are identical - ensure that `git_checkout` updates the permissions to match the checkout target.
| * checkout: take mode into account when comparing index to baselineEdward Thomson2018-03-101-10/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When checking out a file, we determine whether the baseline (what we expect to be in the working directory) actually matches the contents of the working directory. This is safe behavior to prevent us from overwriting changes in the working directory. We look at the index to optimize this test: if we know that the index matches the working directory, then we can simply look at the index data compared to the baseline. We have historically compared the baseline to the index entry by oid. However, we must also compare the mode of the two items to ensure that they are identical. Otherwise, we will refuse to update the working directory for a mode change.
| * diff_tform: fix rename detection with rewrite/delete pairPatrick Steinhardt2018-03-102-1/+216
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A rewritten file can either be classified as a modification of its contents or of a delete of the complete file followed by an addition of the new content. This distinction becomes important when we want to detect renames for rewrites. Given a scenario where a file "a" has been deleted and another file "b" has been renamed to "a", this should be detected as a deletion of "a" followed by a rename of "a" -> "b". Thus, splitting of the original rewrite into a delete/add pair is important here. This splitting is represented by a flag we can set at the current delta. While the flag is already being set in case we want to break rewrites, we do not do so in case where the `GIT_DIFF_FIND_RENAMES_FROM_REWRITES` flag is set. This can trigger an assert when we try to match the source and target deltas. Fix the issue by setting the `GIT_DIFF_FLAG__TO_SPLIT` flag at the delta when it is a rename target and `GIT_DIFF_FIND_RENAMES_FROM_REWRITES` is set.
| * tests: add rename-rewrite scenarios to "renames" repositoryPatrick Steinhardt2018-03-108-0/+15
| | | | | | | | | | | | | | Add two more scenarios to the "renames" repository. The first scenario has a major rewrite of a file and a delete of another file, the second scenario has a deletion of a file and rename of another file to the deleted file. Both scenarios will be used in the following commit.
| * tests: diff::rename: use defines for commit OIDsPatrick Steinhardt2018-03-101-19/+24
| | | | | | | | | | | | | | While we frequently reuse commit OIDs throughout the file, we do not have any constants to refer to these commits. Make this a bit easier to read by giving the commit OIDs somewhat descriptive names of what kind of commit they refer to.
| * merge: virtual commit should be last argument to merge-baseTyrie Vella2018-03-102-4/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Our virtual commit must be the last argument to merge-base: since our algorithm pushes _both_ parents of the virtual commit, it needs to be the last argument, since merge-base: > Given three commits A, B and C, git merge-base A B C will compute the > merge base between A and a hypothetical commit M We want to calculate the merge base between the actual commit ("two") and the virtual commit ("one") - since one actually pushes its parents to the merge-base calculation, we need to calculate the merge base of "two" and the parents of one.
| * Add failing test case for virtual commit merge base issueEdward Thomson2018-03-1027-0/+32
| |
| * merge::trees::recursive: test for virtual base buildingEdward Thomson2018-03-101-0/+25
| | | | | | | | | | Virtual base building: ensure that the virtual base is created and revwalked in the same way as git.
| * merge: reverse merge bases for recursive mergeEdward Thomson2018-03-104-28/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the commits being merged have multiple merge bases, reverse the order when creating the virtual merge base. This is for compatibility with git's merge-recursive algorithm, and ensures that we build identical trees. Git does this to try to use older merge bases first. Per 8918b0c: > It seems to be the only sane way to do it: when a two-head merge is > done, and the merge-base and one of the two branches agree, the > merge assumes that the other branch has something new. > > If we start creating virtual commits from newer merge-bases, and go > back to older merge-bases, and then merge with newer commits again, > chances are that a patch is lost, _because_ the merge-base and the > head agree on it. Unlikely, yes, but it happened to me.
| * oidarray: introduce git_oidarray__reverseEdward Thomson2018-03-102-0/+13
| | | | | | | | Provide a simple function to reverse an oidarray.
| * Introduce additional criss-cross merge branchesEdward Thomson2018-03-1068-0/+11
| |
| * odb: export mempack backendAdrián Medraño Calvo2018-03-102-4/+6
| | | | | | | | Fixes #4492, #4496.
| * Fix unpack double freelhchavez2018-03-103-8/+68
| | | | | | | | | | | | | | | | | | | | | | If an element has been cached, but then the call to packfile_unpack_compressed() fails, the very next thing that happens is that its data is freed and then the element is not removed from the cache, which frees the data again. This change sets obj->data to NULL to avoid the double-free. It also stops trying to resolve deltas after two continuous failed rounds of resolution, and adds a test for this.
| * diff_file: properly refcount blobs when initializing file contentsPatrick Steinhardt2018-03-102-1/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When initializing a `git_diff_file_content` from a source whose data is derived from a blob, we simply assign the blob's pointer to the resulting struct without incrementing its refcount. Thus, the structure can only be used as long as the blob is kept alive by the caller. Fix the issue by using `git_blob_dup` instead of a direct assignment. This function will increment the refcount of the blob without allocating new memory, so it does exactly what we want. As `git_diff_file_content__unload` already frees the blob when `GIT_DIFF_FLAG__FREE_BLOB` is set, we don't need to add new code handling the free but only have to set that flag correctly.
| * stransport: provide error message on trust failuresEtienne Samson2018-03-101-1/+3
| | | | | | Fixes #4440