We are about to vendor in `mimalloc`'s source code which we will want to
include `git-compat-util.h` after defining that constant.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The mingw-w64 GCC seems to link implicitly to libwinpthread, which does
implement a pthread emulation (that is more complete than Git's). Let's
keep preferring Git's.
To avoid linker errors where it thinks that the `pthread_self` and the
`pthread_create` symbols are defined twice, let's give our version a
`win32_` prefix, just like we already do for `pthread_join()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
While Git for Windows does not _ship_ Python (in order to save on
bandwidth), MSYS2 provides very fine Python interpreters that users can
easily take advantage of, by using Git for Windows within its SDK.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch extends the protections introduced for Git GUI's
CVE-2022-41953 to cover `gitk`, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch fixes a vulnerability in Git GUI's "clone" feature
(tracked as CVE-2022-41953) that was graded with a CVSS Score 8.6/10
(high).
These patches were backported to Git GUI in
https://github.com/prati0100/git-gui/pull/85
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Just like CVE-2022-41953 for Git GUI, there exists a vulnerability of
`gitk` where it looks for `taskkill.exe` in the current directory before
searching `PATH`.
Note that the many `exec git` calls are unaffected, due to an obscure
quirk in Tcl's `exec` function. Typically, `git.exe` lives next to
`wish.exe` (i.e. the program that is run to execute `gitk` or Git GUI)
in Git for Windows, and that is the saving grace for `git.exe because
`exec` searches the directory where `wish.exe` lives even before the
current directory, according to
https://www.tcl-lang.org/man/tcl/TclCmd/exec.htm#M24:
If a directory name was not specified as part of the application
name, the following directories are automatically searched in
order when attempting to locate the application:
The directory from which the Tcl executable was loaded.
The current directory.
The Windows 32-bit system directory.
The Windows home directory.
The directories listed in the path.
The same is not true, however, for `taskkill.exe`: it lives in the
Windows system directory (never mind the 32-bit, Tcl's documentation is
outdated on that point, it really means `C:\Windows\system32`).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
As per https://www.tcl.tk/man/tcl8.6/TclCmd/exec.html#M23, Tcl's `exec`
function goes out of its way to imitate the highly dangerous path lookup
of `cmd.exe`, but _of course_ only on Windows:
If a directory name was not specified as part of the application
name, the following directories are automatically searched in
order when attempting to locate the application:
The directory from which the Tcl executable was loaded.
The current directory.
The Windows 32-bit system directory.
The Windows home directory.
The directories listed in the path.
The dangerous part is the second item, of course: `exec` _prefers_
executables in the current directory to those that are actually in the
`PATH`.
It is almost as if people wanted to Windows users vulnerable,
specifically.
To avoid that, Git GUI already has the `_which` function that does not
imitate that dangerous practice when looking up executables in the
search path.
However, Git GUI currently fails to use that function e.g. when trying to
execute `aspell` for spell checking.
That is not only dangerous but combined with Tcl's unfortunate default
behavior and with the fact that Git GUI tries to spell-check a
repository just after cloning, leads to a critical Remote Code Execution
vulnerability.
Let's override both `exec` and `open` to always use `_which` instead of
letting Tcl perform the path lookup, to prevent this attack vector.
This addresses CVE-2022-41953.
For more details, see
https://github.com/git-for-windows/git/security/advisories/GHSA-v4px-mx59-w99c
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We are about to make use of the `_which` function to address
CVE-2022-41953 by overriding Tcl/Tk's unsafe PATH lookup on Windows.
In preparation for that, let's move it close to the top of the file to
make sure that even early `exec` calls that happen during the start-up
of Git GUI benefit from the fix.
This commit is best viewed with `--color-moved`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We need these in `_which` and they should be defined before that
function's definition.
This commit is best viewed with `--color-moved`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The `is_Cygwin` function is used, among other things, to determine
how executables are discovered in the `PATH` list by the `_which` function.
We are about to change the behavior of the `_which` function on Windows
(but not Cygwin): On Windows, we want it to ignore empty elements of the
`PATH` instead of treating them as referring to the current directory
(which is a "legacy feature" according to
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03,
but apparently not explicitly deprecated, the POSIX documentation is
quite unclear on that even if the Cygwin project itself considers it to
be deprecated: https://github.com/cygwin/cygwin/commit/fc74dbf22f5c).
This is important because on Windows, `exec` does something very unsafe
by default (unless we're running a Cygwin version of Tcl, which follows
Unix semantics).
However, we try to `exec` something _inside_ `is_Cygwin` to determine
whether we're running within Cygwin or not, i.e. before we determined
whether we need to handle `PATH` specially or not. That's a Catch-22.
Therefore, and because it is much cleaner anyway, use the
`$::tcl_platform(os)` value which is guaranteed to start with `CYGWIN_`
when running a Cygwin variant of Tcl/Tk, instead of executing `cygpath
--windir`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When looking up an executable via the `_which` function, Git GUI
imitates the `execlp()` strategy where the environment variable `PATH`
is interpreted as a list of paths in which to search.
For historical reasons, stemming from the olden times when it was
uncommon to download a lot of files from the internet into the current
directory, empty elements in this list are treated as if the current
directory had been specified.
Nowadays, of course, this treatment is highly dangerous as the current
directory often contains files that have just been downloaded and not
yet been inspected by the user. Unix/Linux users are essentially
expected to be very, very careful to simply not add empty `PATH`
elements, i.e. not to make use of that feature.
On Windows, however, it is quite common for `PATH` to contain empty
elements by mistake, e.g. as an unintended left-over entry when an
application was installed from the Windows Store and then uninstalled
manually.
While it would probably make most sense to safe-guard not only Windows
users, it seems to be common practice to ignore these empty `PATH`
elements _only_ on Windows, but not on other platforms.
Sadly, this practice is followed inconsistently between different
software projects, where projects with few, if any, Windows-based
contributors tend to be less consistent or even "blissful" about it.
Here is a non-exhaustive list:
Cygwin:
It specifically "eats" empty paths when converting path lists to
POSIX: https://github.com/cygwin/cygwin/commit/753702223c7d
I.e. it follows the common practice.
PowerShell:
It specifically ignores empty paths when searching the `PATH`.
The reason for this is apparently so self-evident that it is not
even mentioned here:
https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables#path-information
I.e. it follows the common practice.
CMD:
Oh my, CMD. Let's just forget about it, nobody in their right
(security) mind takes CMD as inspiration. It is so unsafe by
default that we even planned on dropping `Git CMD` from Git for
Windows altogether, and only walked back on that plan when we
found a super ugly hack, just to keep Git's users secure by
default:
https://github.com/git-for-windows/MINGW-packages/commit/82172388bb51
So CMD chooses to hide behind the battle cry "Works as
Designed!" that all too often leaves users vulnerable. CMD is
probably the most prominent project whose lead you want to avoid
following in matters of security.
Win32 API (`CreateProcess()`)
Just like CMD, `CreateProcess()` adheres to the original design
of the path lookup in the name of backward compatibility (see
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
for details):
If the file name does not contain a directory path, the
system searches for the executable file in the following
sequence:
1. The directory from which the application loaded.
2. The current directory for the parent process.
[...]
I.e. the Win32 API itself chooses backwards compatibility over
users' safety.
Git LFS:
There have been not one, not two, but three security advisories
about Git LFS executing executables from the current directory by
mistake. As part of one of them, a change was introduced to stop
treating empty `PATH` elements as equivalent to `.`:
https://github.com/git-lfs/git-lfs/commit/7cd7bb0a1f0d
I.e. it follows the common practice.
Go:
Go does not follow the common practice, and you can think about
that what you want:
https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go#L114-L135https://github.com/golang/go/blob/go1.19.3/src/path/filepath/path_windows.go#L108-L137
Git Credential Manager:
It tries to imitate Git LFS, but unfortunately misses the empty
`PATH` element handling. As of time of writing, this is in the
process of being fixed:
https://github.com/GitCredentialManager/git-credential-manager/pull/968
So now that we have established that it is a common practice to ignore
empty `PATH` elements on Windows, let's assess this commit's change
using Schneier's Five-Step Process
(https://www.schneier.com/crypto-gram/archives/2002/0415.html#1):
Step 1: What problem does it solve?
It prevents an entire class of Remote Code Execution exploits via
Git GUI's `Clone` functionality.
Step 2: How well does it solve that problem?
Very well. It prevents the attack vector of luring an unsuspecting
victim into cloning an executable into the worktree root directory
that Git GUI immediately executes.
Step 3: What other security problems does it cause?
Maybe non-security problems: If a project (ab-)uses the unsafe
`PATH` lookup. That would not only be unsafe, though, but
fragile in the first place because it would break when running
in a subdirectory. Therefore I would consider this a scenario
not worth keeping working.
Step 4: What are the costs of this measure?
Almost nil, except for the time writing up this commit message
;-)
Step 5: Given the answers to steps two through four, is the security
measure worth the costs?
Yes. Keeping Git's users Secure By Default is worth it. It's a
tiny price to pay compared to the damages even a single
successful exploit can cost.
So let's follow that common practice in Git GUI, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Another (hopefully clean) PR for showing the error warning about atomic
append on windows after failure on APFS, which returns EBADF not EINVAL.
Signed-off-by: David Lomas <dl3@pale-eds.co.uk>
When running Git for Windows on a remote APFS filesystem, it would
appear that the `mingw_open_append()`/`write()` combination would fail
almost exactly like on some CIFS-mounted shares as had been reported in
https://github.com/git-for-windows/git/issues/2753, albeit with a
different `errno` value.
Let's handle that `errno` value just the same, by suggesting to set
`windows.appendAtomically=false`.
Signed-off-by: David Lomas <dl3@pale-eds.co.uk>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
At least on _some_ APFS network shares, Git fails to rename the object
files because they are marked as read-only, because that has the effect
of setting the uchg flag on APFS, which then means the file can't be
renamed or deleted.
To work around that, when a rename failed, and the read-only flag is
set, try to turn it off and on again.
This fixes https://github.com/git-for-windows/git/issues/4482
Signed-off-by: David Lomas <dl3@pale-eds.co.uk>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
"git branch -f X" to repoint the branch X said that X was "checked
out" in another worktree, even when branch X was not and instead
being bisected or rebased. The message was reworded to say the
branch was "in use".
* jc/branch-in-use-error-message:
branch: update the message to refuse touching a branch in-use
"git blame --contents=file" has been taught to work in a bare
repository.
* hy/blame-in-bare-with-contents:
blame: allow --contents to work with bare repo
Command line parser fix, and a small parse-options API update.
* jc/parse-options-short-help:
short help: allow a gap smaller than USAGE_GAP
remote: simplify "remote add --tags" help text
short help: allow multi-line opthelp
Clarify how to pick a starting point for a new topic in the
SubmittingPatches document.
* la/doc-choose-starting-point-fixup:
SubmittingPatches: use of older maintenance tracks is an exception
SubmittingPatches: explain why 'next' and above are inappropriate base
SubmittingPatches: choice of base for fixing an older maintenance track
Rewrite the description of giving a custom command to the
submodule.<name>.update configuration variable.
* pv/doc-submodule-update-settings:
doc: highlight that .gitmodules does not support !command
Fix tests with unportable regex patterns.
* ja/worktree-orphan-fix:
t2400: rewrite regex to avoid unintentional PCRE
builtin/worktree.c: convert tab in advice to space
t2400: drop no-op `--sq` from rev-parse call
The implementation of "get_sha1_hex()" that reads a hexadecimal
string that spells a full object name has been extended to cope
with any hash function used in the repository, but the "sha1" in
its name survived. Rename it to get_hash_hex(), a name that is
more consistent within its friends like get_hash_hex_algop().
* jc/retire-get-sha1-hex:
hex: retire get_sha1_hex()
Clarify how to choose the starting point for a new topic in
developer guidance document.
* la/doc-choose-starting-point:
SubmittingPatches: simplify guidance for choosing a starting point
SubmittingPatches: emphasize need to communicate non-default starting points
SubmittingPatches: de-emphasize branches as starting points
SubmittingPatches: discuss subsystems separately from git.git
SubmittingPatches: reword awkward phrasing
"git branch --list --format=<format>" and friends are taught
a new "%(describe)" placeholder.
* ks/ref-filter-describe:
ref-filter: add new "describe" atom
ref-filter: add multiple-option parsing functions
When the user edits "rebase -i" todo file so that it starts with a
"fixup", which would make it invalid, the command truncated the
rest of the file before giving an error and returning the control
back to the user. Stop truncating to make it easier to correct
such a malformed todo file.
* ah/sequencer-rewrite-todo-fix:
sequencer: finish parsing the todo list despite an invalid first line
Instead of inventing a custom counter variables for debugging,
use existing trace2 facility in the fsync customization codepath.
* bb/use-trace2-counters-for-fsync-stats:
wrapper: use trace2 counters to collect fsync stats
"./configure --with-expat=no" did not work as a way to refuse use
of the expat library on a system with the library installed, which
has been corrected.
* ah/autoconf-fixes:
configure.ac: always save NO_ICONV to config.status
configure.ac: don't overwrite NO_CURL option
configure.ac: don't overwrite NO_EXPAT option
Code simplification.
* jc/tree-walk-drop-base-offset:
tree-walk: drop unused base_offset from do_match()
tree-walk: lose base_offset that is never used in tree_entry_interesting
Finding mistakes in and improving your own patches is a good idea,
but doing so too quickly is being inconsiderate to reviewers who
have just seen the initial iteration and taking their time to review
it. Encourage new developers to perform such a self review before
they send out their patches, not after. After sending a patch that
they immediately found mistakes in, they are welcome to comment on
them, mentioning what and how they plan to improve them in an
updated version, before sending out their updates.
Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Command line parser fixes.
* jc/parse-options-show-branch:
show-branch: reject --[no-](topo|date)-order
show-branch: --no-sparse should give dense output
While we could technically fix each and every bug on top of the
commit that introduced it, it is not necessarily practical. For
trivial and low-value bugfixes, it often is simpler and sufficient
to just fix it in the current maintenance track, leaving the bug
unfixed in the older maintenance tracks.
Demote the "use older maintenance track to fix old bugs" as a side
note, and explain that the choice is used only in exceptional cases.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'next' branch is primarily meant to be a testing ground to make
sure that topics that are reasonably well done work well together.
Building a new work on it would mean everything that was already in
'next' must have graduated to 'master' before the new work can also
be merged to 'master', and that is why we do not encourage basing
new work on 'next'.
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace all cases of `\s` with ` ` as it is not part of POSIX BRE or ERE
and therefore not all versions of grep handle it.
For the same reason all cases of `\S` are replaced with `[^ ]`. It is
not an exact replacement but it is close enough for this use case.
Also, do not write `\+` in BRE and expect it to mean 1 or more;
it is a GNU extension that may not work everywhere.
Remove `.*` from the end of a pattern that is not right-anchored.
Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
