summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2019-12-05Merge branch 'sg/unpack-progress-throughput'Junio C Hamano1-1/+2
"git unpack-objects" used to show progress based only on the number of received and unpacked objects, which stalled when it has to handle an unusually large object. It now shows the throughput as well. * sg/unpack-progress-throughput: builtin/unpack-objects.c: show throughput progress
2019-12-05Merge branch 'pb/submodule-update-fetches'Junio C Hamano1-1/+2
Doc update. * pb/submodule-update-fetches: doc: mention that 'git submodule update' fetches missing commits
2019-12-05Merge branch 'jc/azure-ci-osx-fix-fix'Junio C Hamano1-1/+2
CI jobs for macOS has been made less chatty when updating perforce package used during testing. * jc/azure-ci-osx-fix-fix: ci(osx): update homebrew-cask repository with less noise
2019-12-05Merge branch 'dl/range-diff-with-notes'Junio C Hamano10-50/+291
"git range-diff" learned to take the "--notes=<ref>" and the "--no-notes" options to control the commit notes included in the log message that gets compared. * dl/range-diff-with-notes: format-patch: pass notes configuration to range-diff range-diff: pass through --notes to `git log` range-diff: output `## Notes ##` header t3206: range-diff compares logs with commit notes t3206: s/expected/expect/ t3206: disable parameter substitution in heredoc t3206: remove spaces after redirect operators pretty-options.txt: --notes accepts a ref instead of treeish rev-list-options.txt: remove reference to --show-notes argv-array: add space after `while`
2019-12-05Merge branch 'jh/userdiff-python-async'Junio C Hamano7-1/+32
The userdiff machinery has been taught that "async def" is another way to begin a "function" in Python. * jh/userdiff-python-async: userdiff: support Python async functions
2019-12-05Merge branch 'ec/fetch-mark-common-refs-trace2'Junio C Hamano1-1/+12
Trace2 annotation. * ec/fetch-mark-common-refs-trace2: fetch: add trace2 instrumentation
2019-12-05Merge branch 'dd/rebase-merge-reserves-onto-label'Junio C Hamano2-0/+26
The logic to avoid duplicate label names generated by "git rebase --rebase-merges" forgot that the machinery itself uses "onto" as a label name, which must be avoided by auto-generated labels, which has been corrected. * dd/rebase-merge-reserves-onto-label: sequencer: handle rebase-merges for "onto" message
2019-12-05Merge branch 'js/builtin-add-i'Junio C Hamano9-23/+743
The beginning of rewriting "git add -i" in C. * js/builtin-add-i: built-in add -i: implement the `help` command built-in add -i: use color in the main loop built-in add -i: support `?` (prompt help) built-in add -i: show unique prefixes of the commands built-in add -i: implement the main loop built-in add -i: color the header in the `status` command built-in add -i: implement the `status` command diff: export diffstat interface Start to implement a built-in version of `git add --interactive`
2019-12-05Merge branch 'js/rebase-r-safer-label'Junio C Hamano2-27/+51
A label used in the todo list that are generated by "git rebase --rebase-merges" is used as a part of a refname; the logic to come up with the label has been tightened to avoid names that cannot be used as such. * js/rebase-r-safer-label: rebase -r: let `label` generate safer labels rebase-merges: move labels' whitespace mangling into `label_oid()`
2019-12-05Merge branch 'ep/guard-kset-tar-headers'Junio C Hamano2-0/+9
Code clean-up. * ep/guard-kset-tar-headers: kset.h, tar.h: add missing header guard to prevent multiple inclusion
2019-12-06Merge branch 'jg/revert-untracked'Pratyush Yadav8-225/+889
git-gui learned to delete untracked files when the "Revert Changes" option is selected. Since there are two types of revert operations (one for tracked files and one for untracked ones), the "checkout" and "deletion" operations are done in parallel. The status bar is updated to allow both to use it in parallel. * jg/revert-untracked: git-gui: revert untracked files by deleting them git-gui: update status bar to track operations git-gui: consolidate naming conventions
2019-12-06git-gui: revert untracked files by deleting themJonathan Gilbert3-82/+500
Update the revert_helper proc to check for untracked files as well as changes, and then handle changes to be reverted and untracked files with independent blocks of code. Prompt the user independently for untracked files, since the underlying action is fundamentally different (rm -f). If after deleting untracked files, the directory containing them becomes empty, then remove the directory as well. Migrate unlocking of the index out of _close_updateindex to a responsibility of the caller, to permit paths that don't directly unlock the index, and refactor the error handling added in d4e890e5 so that callers can make flow control decisions in the event of errors. Update Tcl/Tk dependency from 8.4 to 8.6 in git-gui.sh. A new proc delete_files takes care of actually deleting the files in batches, using the Tcler's Wiki recommended approach for keeping the UI responsive. Since the checkout_index and delete_files calls are both asynchronous and could potentially complete in any order, a "chord" is used to coordinate unlocking the index and returning the UI to a usable state only after both operations are complete. The `SimpleChord` class, based on TclOO (Tcl/Tk 8.6), is added in this commit. Signed-off-by: Jonathan Gilbert <JonathanG@iQmetrix.com> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2019-12-06git-gui: update status bar to track operationsJonathan Gilbert7-108/+354
Update the status bar to track updates as individual "operations" that can overlap. Update all call sites to interact with the new status bar mechanism. Update initialization to explicitly clear status text, since otherwise it may persist across future operations. Signed-off-by: Jonathan Gilbert <JonathanG@iQmetrix.com> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2019-12-06git-gui: consolidate naming conventionsJonathan Gilbert1-46/+46
A few variables in this file use camelCase, while the overall standard is snake_case. A consistent naming scheme will improve readability of future changes. To avoid mixing naming changes with semantic changes, this commit contains only naming changes. Signed-off-by: Jonathan Gilbert <JonathanG@iQmetrix.com> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2019-12-05xdiff: unignore changes in function contextRené Scharfe2-5/+18
Changes involving only blank lines are hidden with --ignore-blank-lines, unless they appear in the context lines of other changes. This is handled by xdl_get_hunk() for context added by --inter-hunk-context, -u and -U. Function context for -W and --function-context added by xdl_emit_diff() doesn't pay attention to such ignored changes; it relies fully on xdl_get_hunk() and shows just the post-image of ignored changes appearing in function context. That's inconsistent and confusing. Improve the result of using --ignore-blank-lines and --function-context together by fully showing ignored changes if they happen to fall within function context. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05Merge branch 'win32-accommodate-funny-drive-names'Johannes Schindelin4-6/+54
While the only permitted drive letters for physical drives on Windows are letters of the US-English alphabet, this restriction does not apply to virtual drives assigned via `subst <letter>: <path>`. To prevent targeted attacks against systems where "funny" drive letters such as `1` or `!` are assigned, let's handle them as regular drive letters on Windows. This fixes CVE-2019-1351. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Merge branch 'win32-filenames-cannot-have-trailing-spaces-or-periods'Johannes Schindelin11-4/+127
On Windows, filenames cannot have trailing spaces or periods, when opening such paths, they are stripped automatically. Read: you can open the file `README` via the file name `README . . .`. This ambiguity can be used in combination with other security bugs to cause e.g. remote code execution during recursive clones. This patch series fixes that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Merge branch 'fix-mingw-quoting-bug'Johannes Schindelin3-5/+156
This patch fixes a vulnerability in the Windows-specific code where a submodule names ending in a backslash were quoted incorrectly, and that bug could be abused to insert command-line parameters e.g. to `ssh` in a recursive clone. Note: this bug is Windows-only, as we have to construct a command line for the process-to-spawn, unlike Linux/macOS, where `execv()` accepts an already-split command line. While at it, other quoting issues are fixed as well. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Merge branch 'dubiously-nested-submodules'Johannes Schindelin4-2/+79
Recursive clones are currently affected by a vulnerability that is caused by too-lax validation of submodule names. This topic branch fixes that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Merge branch 'turn-on-protectntfs-by-default'Johannes Schindelin3-28/+31
This patch series makes it safe to use Git on Windows drives, even if running on a mounted network share or within the Windows Subsystem for Linux (WSL). This topic branch addresses CVE-2019-1353. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05mingw: handle `subst`-ed "DOS drives"Johannes Schindelin4-3/+36
Over a decade ago, in 25fe217b86c (Windows: Treat Windows style path names., 2008-03-05), Git was taught to handle absolute Windows paths, i.e. paths that start with a drive letter and a colon. Unbeknownst to us, while drive letters of physical drives are limited to letters of the English alphabet, there is a way to assign virtual drive letters to arbitrary directories, via the `subst` command, which is _not_ limited to English letters. It is therefore possible to have absolute Windows paths of the form `1:\what\the\hex.txt`. Even "better": pretty much arbitrary Unicode letters can also be used, e.g. `ä:\tschibät.sch`. While it can be sensibly argued that users who set up such funny drive letters really seek adverse consequences, the Windows Operating System is known to be a platform where many users are at the mercy of administrators who have their very own idea of what constitutes a reasonable setup. Therefore, let's just make sure that such funny paths are still considered absolute paths by Git, on Windows. In addition to Unicode characters, pretty much any character is a valid drive letter, as far as `subst` is concerned, even `:` and `"` or even a space character. While it is probably the opposite of smart to use them, let's safeguard `is_dos_drive_prefix()` against all of them. Note: `[::1]:repo` is a valid URL, but not a valid path on Windows. As `[` is now considered a valid drive letter, we need to be very careful to avoid misinterpreting such a string as valid local path in `url_is_local_not_ssh()`. To do that, we use the just-introduced function `is_valid_path()` (which will label the string as invalid file name because of the colon characters). This fixes CVE-2019-1351. Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Merge branch 'disallow-dotgit-via-ntfs-alternate-data-streams'Johannes Schindelin6-8/+162
This patch series plugs an attack vector we had overlooked in our December 2014 work on `core.protectNTFS`. Essentially, the path `.git::$INDEX_ALLOCATION/config` is interpreted as `.git/config` when NTFS Alternate Data Streams are available (which they are on Windows, and at least on network shares that are SMB-mounted on macOS). Needless to say: we don't want that. In fact, we want to stay on the very safe side and not even special-case the `$INDEX_ALLOCATION` stream type: let's just prevent Git from touching _any_ explicitly specified Alternate Data Stream of `.git`. In essence, we'll prevent Git from tracking, or writing to, any path with a segment of the form `.git:<anything>`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05mingw: refuse to access paths with trailing spaces or periodsJohannes Schindelin8-2/+123
When creating a directory on Windows whose path ends in a space or a period (or chains thereof), the Win32 API "helpfully" trims those. For example, `mkdir("abc ");` will return success, but actually create a directory called `abc` instead. This stems back to the DOS days, when all file names had exactly 8 characters plus exactly 3 characters for the file extension, and the only way to have shorter names was by padding with spaces. Sadly, this "helpful" behavior is a bit inconsistent: after a successful `mkdir("abc ");`, a `mkdir("abc /def")` will actually _fail_ (because the directory `abc ` does not actually exist). Even if it would work, we now have a serious problem because a Git repository could contain directories `abc` and `abc `, and on Windows, they would be "merged" unintentionally. As these paths are illegal on Windows, anyway, let's disallow any accesses to such paths on that Operating System. For practical reasons, this behavior is still guarded by the config setting `core.protectNTFS`: it is possible (and at least two regression tests make use of it) to create commits without involving the worktree. In such a scenario, it is of course possible -- even on Windows -- to create such file names. Among other consequences, this patch disallows submodules' paths to end in spaces on Windows (which would formerly have confused Git enough to try to write into incorrect paths, anyway). While this patch does not fix a vulnerability on its own, it prevents an attack vector that was exploited in demonstrations of a number of recently-fixed security bugs. The regression test added to `t/t7417-submodule-path-url.sh` reflects that attack vector. Note that we have to adjust the test case "prevent git~1 squatting on Windows" in `t/t7415-submodule-names.sh` because of a very subtle issue. It tries to clone two submodules whose names differ only in a trailing period character, and as a consequence their git directories differ in the same way. Previously, when Git tried to clone the second submodule, it thought that the git directory already existed (because on Windows, when you create a directory with the name `b.` it actually creates `b`), but with this patch, the first submodule's clone will fail because of the illegal name of the git directory. Therefore, when cloning the second submodule, Git will take a different code path: a fresh clone (without an existing git directory). Both code paths fail to clone the second submodule, both because the the corresponding worktree directory exists and is not empty, but the error messages are worded differently. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05quote-stress-test: offer to test quoting arguments for MSYS2 shJohannes Schindelin1-3/+10
It is unfortunate that we need to quote arguments differently on Windows, depending whether we build a command-line for MSYS2's `sh` or for other Windows executables. We already have a test helper to verify the latter, with this patch we can also verify the former. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05mingw: refuse to access paths with illegal charactersJohannes Schindelin3-3/+18
Certain characters are not admissible in file names on Windows, even if Cygwin/MSYS2 (and therefore, Git for Windows' Bash) pretend that they are, e.g. `:`, `<`, `>`, etc Let's disallow those characters explicitly in Windows builds of Git. Note: just like trailing spaces or periods, it _is_ possible on Windows to create commits adding files with such illegal characters, as long as the operation leaves the worktree untouched. To allow for that, we continue to guard `is_valid_win32_path()` behind the config setting `core.protectNTFS`, so that users _can_ continue to do that, as long as they turn the protections off via that config setting. Among other problems, this prevents Git from trying to write to an "NTFS Alternate Data Stream" (which refers to metadata stored alongside a file, under a special name: "<filename>:<stream-name>"). This fix therefore also prevents an attack vector that was exploited in demonstrations of a number of recently-fixed security bugs. Further reading on illegal characters in Win32 filenames: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05unpack-trees: let merged_entry() pass through do_add_entry()'s errorsJohannes Schindelin1-1/+2
A `git clone` will end with exit code 0 when `merged_entry()` returns a positive value during a call of `unpack_trees()` to `traverse_trees()`. The reason is that `unpack_trees()` will interpret a positive value not to be an error. The problem is, however, that `add_index_entry()` (which is called by `merged_entry()` can report an error, and we really should fail the entire clone in such a case. Let's fix this problem, in preparation for a Windows-specific patch disallowing `mkdir()` with directory names that contain a trailing space (which is illegal on NTFS): we want `git clone` to abort when a path cannot be checked out due to that condition. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05quote-stress-test: allow skipping some trialsJohannes Schindelin1-1/+5
When the, say, 93rd trial run fails, it is a good idea to have a way to skip the first 92 trials and dig directly into the 93rd in a debugger. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05t6130/t9350: prepare for stringent Win32 path validationJohannes Schindelin2-1/+2
On Windows, file names cannot contain asterisks nor newline characters. In an upcoming commit, we will make this limitation explicit, disallowing even the creation of commits that introduce such file names. However, in the test scripts touched by this patch, we _know_ that those paths won't be checked out, so we _want_ to allow such file names. Happily, the stringent path validation will be guarded via the `core.protectNTFS` flag, so all we need to do is to force that flag off temporarily. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05quote-stress-test: accept arguments to test via the command-lineJohannes Schindelin1-11/+20
When the stress test reported a problem with quoting certain arguments, it is helpful to have a facility to play with those arguments in order to find out whether variations of those arguments are affected, too. Let's allow `test-run-command quote-stress-test -- <args>` to be used for that purpose. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05tests: add a helper to stress test argument quotingGarima Singh1-2/+116
On Windows, we have to do all the command-line argument quoting ourselves. Worse: we have to have two versions of said quoting, one for MSYS2 programs (which have their own dequoting rules) and the rest. We care mostly about the rest, and to make sure that that works, let's have a stress test that comes up with all kinds of awkward arguments, verifying that a spawned sub-process receives those unharmed. Signed-off-by: Garima Singh <garima.singh@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05Disallow dubiously-nested submodule git directoriesJohannes Schindelin4-2/+79
Currently it is technically possible to let a submodule's git directory point right into the git dir of a sibling submodule. Example: the git directories of two submodules with the names `hippo` and `hippo/hooks` would be `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively, but the latter is already intended to house the former's hooks. In most cases, this is just confusing, but there is also a (quite contrived) attack vector where Git can be fooled into mistaking remote content for file contents it wrote itself during a recursive clone. Let's plug this bug. To do so, we introduce the new function `validate_submodule_git_dir()` which simply verifies that no git dir exists for any leading directories of the submodule name (if there are any). Note: this patch specifically continues to allow sibling modules names of the form `core/lib`, `core/doc`, etc, as long as `core` is not a submodule name. This fixes CVE-2019-1387. Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05protect_ntfs: turn on NTFS protection by defaultJohannes Schindelin2-3/+1
Back in the DOS days, in the FAT file system, file names always consisted of a base name of length 8 plus a file extension of length 3. Shorter file names were simply padded with spaces to the full 8.3 format. Later, the FAT file system was taught to support _also_ longer names, with an 8.3 "short name" as primary file name. While at it, the same facility allowed formerly illegal file names, such as `.git` (empty base names were not allowed), which would have the "short name" `git~1` associated with it. For backwards-compatibility, NTFS supports alternative 8.3 short filenames, too, even if starting with Windows Vista, they are only generated on the system drive by default. We addressed the problem that the `.git/` directory can _also_ be accessed via `git~1/` (when short names are enabled) in 2b4c6efc821 (read-cache: optionally disallow NTFS .git variants, 2014-12-16), i.e. since Git v1.9.5, by introducing the config setting `core.protectNTFS` and enabling it by default on Windows. In the meantime, Windows 10 introduced the "Windows Subsystem for Linux" (short: WSL), i.e. a way to run Linux applications/distributions in a thinly-isolated subsystem on Windows (giving rise to many a "2016 is the Year of Linux on the Desktop" jokes). WSL is getting increasingly popular, also due to the painless way Linux application can operate directly ("natively") on files on Windows' file system: the Windows drives are mounted automatically (e.g. `C:` as `/mnt/c/`). Taken together, this means that we now have to enable the safe-guards of Git v1.9.5 also in WSL: it is possible to access a `.git` directory inside `/mnt/c/` via the 8.3 name `git~1` (unless short name generation was disabled manually). Since regular Linux distributions run in WSL, this means we have to enable `core.protectNTFS` at least on Linux, too. To enable Services for Macintosh in Windows NT to store so-called resource forks, NTFS introduced "Alternate Data Streams". Essentially, these constitute additional metadata that are connected to (and copied with) their associated files, and they are accessed via pseudo file names of the form `filename:<stream-name>:<stream-type>`. In a recent patch, we extended `core.protectNTFS` to also protect against accesses via NTFS Alternate Data Streams, e.g. to prevent contents of the `.git/` directory to be "tracked" via yet another alternative file name. While it is not possible (at least by default) to access files via NTFS Alternate Data Streams from within WSL, the defaults on macOS when mounting network shares via SMB _do_ allow accessing files and directories in that way. Therefore, we need to enable `core.protectNTFS` on macOS by default, too, and really, on any Operating System that can mount network shares via SMB/CIFS. A couple of approaches were considered for fixing this: 1. We could perform a dynamic NTFS check similar to the `core.symlinks` check in `init`/`clone`: instead of trying to create a symbolic link in the `.git/` directory, we could create a test file and try to access `.git/config` via 8.3 name and/or Alternate Data Stream. 2. We could simply "flip the switch" on `core.protectNTFS`, to make it "on by default". The obvious downside of 1. is that it won't protect worktrees that were clone with a vulnerable Git version already. We considered patching code paths that check out files to check whether we're running on an NTFS system dynamically and persist the result in the repository-local config setting `core.protectNTFS`, but in the end decided that this solution would be too fragile, and too involved. The obvious downside of 2. is that everybody will have to "suffer" the performance penalty incurred from calling `is_ntfs_dotgit()` on every path, even in setups where. After the recent work to accelerate `is_ntfs_dotgit()` in most cases, it looks as if the time spent on validating ten million random file names increases only negligibly (less than 20ms, well within the standard deviation of ~50ms). Therefore the benefits outweigh the cost. Another downside of this is that paths that might have been acceptable previously now will be forbidden. Realistically, though, this is an improvement because public Git hosters already would reject any `git push` that contains such file names. Note: There might be a similar problem mounting HFS+ on Linux. However, this scenario has been considered unlikely and in light of the cost (in the aforementioned benchmark, `core.protectHFS = true` increased the time from ~440ms to ~610ms), it was decided _not_ to touch the default of `core.protectHFS`. This change addresses CVE-2019-1353. Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> Helped-by: Garima Singh <garima.singh@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05path: also guard `.gitmodules` against NTFS Alternate Data StreamsJohannes Schindelin2-2/+7
We just safe-guarded `.git` against NTFS Alternate Data Stream-related attack vectors, and now it is time to do the same for `.gitmodules`. Note: In the added regression test, we refrain from verifying all kinds of variations between short names and NTFS Alternate Data Streams: as the new code disallows _all_ Alternate Data Streams of `.gitmodules`, it is enough to test one in order to know that all of them are guarded against. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05mingw: fix quoting of argumentsJohannes Schindelin2-3/+20
We need to be careful to follow proper quoting rules. For example, if an argument contains spaces, we have to quote them. Double-quotes need to be escaped. Backslashes need to be escaped, but only if they are followed by a double-quote character. We need to be _extra_ careful to consider the case where an argument ends in a backslash _and_ needs to be quoted: in this case, we append a double-quote character, i.e. the backslash now has to be escaped! The current code, however, fails to recognize that, and therefore can turn an argument that ends in a single backslash into a quoted argument that now ends in an escaped double-quote character. This allows subsequent command-line parameters to be split and part of them being mistaken for command-line options, e.g. through a maliciously-crafted submodule URL during a recursive clone. Technically, we would not need to quote _all_ arguments which end in a backslash _unless_ the argument needs to be quoted anyway. For example, `test\` would not need to be quoted, while `test \` would need to be. To keep the code simple, however, and therefore easier to reason about and ensure its correctness, we now _always_ quote an argument that ends in a backslash. This addresses CVE-2019-1350. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05is_ntfs_dotgit(): speed it upJohannes Schindelin1-25/+30
Previously, this function was written without focusing on speed, intending to make reviewing the code as easy as possible, to avoid any bugs in this critical code. Turns out: we can do much better on both accounts. With this patch, we make it as fast as this developer can make it go: - We avoid the call to `is_dir_sep()` and make all the character comparisons explicit. - We avoid the cost of calling `strncasecmp()` and unroll the test for `.git` and `git~1`, not even using `tolower()` because it is faster to compare against two constant values. - We look for `.git` and `.git~1` first thing, and return early if not found. - We also avoid calling a separate function for detecting chains of spaces and periods. Each of these improvements has a noticeable impact on the speed of `is_ntfs_dotgit()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05path: safeguard `.git` against NTFS Alternate Streams AccessesJohannes Schindelin2-1/+12
Probably inspired by HFS' resource streams, NTFS supports "Alternate Data Streams": by appending `:<stream-name>` to the file name, information in addition to the file contents can be written and read, information that is copied together with the file (unless copied to a non-NTFS location). These Alternate Data Streams are typically used for things like marking an executable as having just been downloaded from the internet (and hence not necessarily being trustworthy). In addition to a stream name, a stream type can be appended, like so: `:<stream-name>:<stream-type>`. Unless specified, the default stream type is `$DATA` for files and `$INDEX_ALLOCATION` for directories. In other words, `.git::$INDEX_ALLOCATION` is a valid way to reference the `.git` directory! In our work in Git v2.2.1 to protect Git on NTFS drives under `core.protectNTFS`, we focused exclusively on NTFS short names, unaware of the fact that NTFS Alternate Data Streams offer a similar attack vector. Let's fix this. Seeing as it is better to be safe than sorry, we simply disallow paths referring to *any* NTFS Alternate Data Stream of `.git`, not just `::$INDEX_ALLOCATION`. This also simplifies the implementation. This closes CVE-2019-1352. Further reading about NTFS Alternate Data Streams: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/c54dec26-1551-4d3a-a0ea-4fa40f848eb3 Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05is_ntfs_dotgit(): only verify the leading segmentJohannes Schindelin3-5/+19
The config setting `core.protectNTFS` is specifically designed to work not only on Windows, but anywhere, to allow for repositories hosted on, say, Linux servers to be protected against NTFS-specific attack vectors. As a consequence, `is_ntfs_dotgit()` manually splits backslash-separated paths (but does not do the same for paths separated by forward slashes), under the assumption that the backslash might not be a valid directory separator on the _current_ Operating System. However, the two callers, `verify_path()` and `fsck_tree()`, are supposed to feed only individual path segments to the `is_ntfs_dotgit()` function. This causes a lot of duplicate scanning (and very inefficient scanning, too, as the inner loop of `is_ntfs_dotgit()` was optimized for readability rather than for speed. Let's simplify the design of `is_ntfs_dotgit()` by putting the burden of splitting the paths by backslashes as directory separators on the callers of said function. Consequently, the `verify_path()` function, which already splits the path by directory separators, now treats backslashes as directory separators _explicitly_ when `core.protectNTFS` is turned on, even on platforms where the backslash is _not_ a directory separator. Note that we have to repeat some code in `verify_path()`: if the backslash is not a directory separator on the current Operating System, we want to allow file names like `\`, but we _do_ want to disallow paths that are clearly intended to cause harm when the repository is cloned on Windows. The `fsck_tree()` function (the other caller of `is_ntfs_dotgit()`) now needs to look for backslashes in tree entries' names specifically when `core.protectNTFS` is turned on. While it would be tempting to completely disallow backslashes in that case (much like `fsck` reports names containing forward slashes as "full paths"), this would be overzealous: when `core.protectNTFS` is turned on in a non-Windows setup, backslashes are perfectly valid characters in file names while we _still_ want to disallow tree entries that are clearly designed to exploit NTFS-specific behavior. This simplification will make subsequent changes easier to implement, such as turning `core.protectNTFS` on by default (not only on Windows) or protecting against attack vectors involving NTFS Alternate Data Streams. Incidentally, this change allows for catching malicious repositories that contain tree entries of the form `dir\.gitmodules` already on the server side rather than only on the client side (and previously only on Windows): in contrast to `is_ntfs_dotgit()`, the `is_ntfs_dotgitmodules()` function already expects the caller to split the paths by directory separators. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05test-path-utils: offer to run a protectNTFS/protectHFS benchmarkGarima Singh1-0/+96
In preparation to flipping the default on `core.protectNTFS`, let's have some way to measure the speed impact of this config setting reliably (and for comparison, the `core.protectHFS` config setting). For now, this is a manual performance benchmark: ./t/helper/test-path-utils protect_ntfs_hfs [arguments...] where the arguments are an optional number of file names to test with, optionally followed by minimum and maximum length of the random file names. The default values are one million, 3 and 20, respectively. Just like `sqrti()` in `bisect.c`, we introduce a very simple function to approximation the square root of a given value, in order to avoid having to introduce the first user of `<math.h>` in Git's source code. Note: this is _not_ implemented as a Unix shell script in t/perf/ because we really care about _very_ precise timings here, and Unix shell scripts are simply unsuited for precise and consistent benchmarking. Signed-off-by: Garima Singh <garima.singh@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-12-05rebase: fix format.useAutoBase breakageDenton Liu2-2/+3
With `format.useAutoBase = true`, running rebase resulted in an error: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base=<base-commit-id> manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Fix this by always passing `--no-base` to format-patch from rebase so that the effect of `format.useAutoBase` is negated. Reported-by: Christian Biesinger <cbiesinger@google.com> Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05format-patch: teach --no-baseDenton Liu3-4/+14
If `format.useAutoBase = true`, there was no way to override this from the command-line. Teach the `--no-base` option in format-patch to override `format.useAutoBase`. Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05t4014: use test_config()Denton Liu1-5/+3
Instead of manually unsetting the config after the test case is done, use test_config() to do it automatically. While we're at it, fix a typo in a test case name. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05format-patch: fix indentationDenton Liu1-1/+1
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05t3400: demonstrate failure with format.useAutoBaseDenton Liu1-0/+6
Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when `format.useAutoBase = true`. It fails when rebasing a branch: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base=<base-commit-id> manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Demonstrate that failure here. Reported-by: Christian Biesinger <cbiesinger@google.com> Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04t7700: stop losing return codes of git commandsDenton Liu1-10/+9
In a pipe, only the return code of the last command is used. Thus, all other commands will have their return codes masked. Rewrite pipes so that there are no git commands upstream so that we will know if a command fails. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04t7700: make references to SHA-1 genericDenton Liu1-19/+19
Make the test more hash-agnostic by renaming variables from "sha1" to some variation of "oid" or "packid". Also, replace the regex, `[0-9a-f]\{40\}` with `$OID_REGEX`. A better name for "incrpackid" (incremental pack-id) might have been just "packid". However, later in the test suite, we have other uses of "packid". Although the scopes of these variables don't conflict, a future developer may think that commit_and_pack() and test_has_duplicate_object() are semantically related somehow since they share the same variable name. Give them distinct names so that it's clear these uses are unrelated. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04t7700: replace egrep with grepDenton Liu1-3/+3
The egrep expressions in this test suite were of the form `^$variable`. Although egrep works just fine, it's overkill since we're not using any extended regex. Replace egrep invocations with grep so that we aren't swatting flies with a sledgehammer. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04t7700: consolidate code into test_has_duplicate_object()Denton Liu1-48/+23
The code to test that objects were not duplicated from the packfile was duplicated many times. Extract the duplicated code into test_has_duplicate_object() and use that instead. Refactor the resulting extraction so that if the git command fails, the return code is not silently lost. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04t7700: consolidate code into test_no_missing_in_packs()Denton Liu1-40/+15
The code to test that objects were not missing from the packfile was duplicated many times. Extract the duplicated code into test_no_missing_in_packs() and use that instead. Refactor the resulting extraction so that if any git commands fail, their return codes are not silently lost. Instead of verifying each file of `alt_objects/pack/*.idx` individually in a for-loop, batch them together into one verification step. The original testing construct was O(n^2): it used a grep in a loop to test whether any objects were missing in the packfile. Rewrite this to extract the hash using sed or cut, sort the files, then use `comm -23` so that finding missing lines from the original file is done more efficiently. While we're at it, add a space to `commit_and_pack ()` for style. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04doc: replace LKML link with lore.kernel.orgDenton Liu1-1/+1
Since we're now recommending lore.kernel.org, replace LKML link with lore.kernel.org. Although LKML has been around for a long time, nothing lasts forever (see Gmane). Since LKML uses opaque message identifiers, switching to lore.kernel.org should be a strict improvement since, even if lore.kernel.org goes down, the Message-ID will allow future readers to look up the referenced messages on any other archive. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-04RelNotes: replace Gmane with real Message-IDsDenton Liu9-10/+11
The only references to Gmane that remain are in RelNotes. Although these are definitely not in active use, they might be of historical interest for future readers so let's ensure that mail references are more robust. Replace links to Gmane with links to lore.kernel.org (which is our new preferred mailing list archive and has the Message-ID in the URL) and bare Gmane ID references with Message-IDs. The Message-IDs were found by searching for "gmane:<id>" on https://public-inbox.org/git/ and taking the resulting message. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>