Taylor Blau [Wed, 28 May 2025 19:17:05 +0000 (15:17 -0400)]
Sync with 2.47.3
* maint-2.47:
Git 2.47.3
Git 2.46.4
Git 2.45.4
Git 2.44.4
Git 2.43.7
wincred: avoid buffer overflow in wcsncat()
bundle-uri: fix arbitrary file writes via parameter injection
config: quote values containing CR character
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
Taylor Blau [Wed, 28 May 2025 18:59:31 +0000 (14:59 -0400)]
Sync with 2.46.4
* maint-2.46:
Git 2.46.4
Git 2.45.4
Git 2.44.4
Git 2.43.7
wincred: avoid buffer overflow in wcsncat()
bundle-uri: fix arbitrary file writes via parameter injection
config: quote values containing CR character
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
Taylor Blau [Wed, 28 May 2025 18:57:08 +0000 (14:57 -0400)]
Sync with 2.45.4
* maint-2.45:
Git 2.45.4
Git 2.44.4
Git 2.43.7
wincred: avoid buffer overflow in wcsncat()
bundle-uri: fix arbitrary file writes via parameter injection
config: quote values containing CR character
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
Taylor Blau [Wed, 28 May 2025 18:51:38 +0000 (14:51 -0400)]
Sync with 2.44.4
* maint-2.44:
Git 2.44.4
Git 2.43.7
wincred: avoid buffer overflow in wcsncat()
bundle-uri: fix arbitrary file writes via parameter injection
config: quote values containing CR character
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
Taylor Blau [Wed, 28 May 2025 18:47:12 +0000 (14:47 -0400)]
Sync with 2.43.7
* maint-2.43:
Git 2.43.7
wincred: avoid buffer overflow in wcsncat()
bundle-uri: fix arbitrary file writes via parameter injection
config: quote values containing CR character
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
Taylor Blau [Mon, 19 May 2025 22:30:29 +0000 (18:30 -0400)]
wincred: avoid buffer overflow in wcsncat()
The wincred credential helper uses a static buffer ("target") as a
unique key for storing and comparing against internal storage. It does
this by building up a string is supposed to look like:
git:$PROTOCOL://$USERNAME@$HOST/@PATH
However, the static "target" buffer is declared as a wide string with no
more than 1,024 wide characters. The first call to wcsncat() is almost
correct (it copies no more than ARRAY_SIZE(target) wchar_t's), but does
not account for the trailing NUL, introducing an off-by-one error.
But subsequent calls to wcsncat() have an additional problem on top of
the off-by-one. They do not account for the length of the existing
wide string being built up in 'target'. So the following:
$ perl -e '
my $x = "x" x 1_000;
print "protocol=$x\nhost=$x\nusername=$x\npath=$x\n"
' |
C\:/Program\ Files/Git/mingw64/libexec/git-core/git-credential-wincred.exe get
will result in a segmentation fault from over-filling buffer.
This bug is as old as the wincred helper itself, dating back to a6253da0f3 (contrib: add win32 credential-helper, 2012-07-27). Commit 8b2d219a3d (wincred: improve compatibility with windows versions,
2013-01-10) replaced the use of strncat() with wcsncat(), but retained
the buggy behavior.
Fix this by using a "target_append()" helper which accounts for both the
length of the existing string within the buffer, as well as the trailing
NUL character.
Reported-by: David Leadbeater <dgl@dgl.cx> Helped-by: David Leadbeater <dgl@dgl.cx> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Taylor Blau [Fri, 23 May 2025 21:17:06 +0000 (17:17 -0400)]
Merge branch 'js/gitk-git-gui-harden-exec-open' into maint-2.43
This merges in fixes for CVE-2025-27614, CVE-2025-27613, CVE-2025-46334,
and CVE-2025-46835 targeting Gitk and Git GUI.
* js/gitk-git-gui-harden-exec-open: (41 commits)
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: override exec and open only on Windows
gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
git-gui: use git_read in githook_read
git-gui: sanitize $PATH on all platforms
git-gui: break out a separate function git_read_nice
git-gui: assure PATH has only absolute elements.
git-gui: remove option --stderr from git_read
git-gui: cleanup git-bash menu item
git-gui: sanitize 'exec' arguments: background
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: sanitize 'exec' arguments: simple cases
git-gui: avoid auto_execok for git-bash menu item
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove unused proc is_shellscript
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: use only the configured shell
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
gitk: encode arguments correctly with "open"
gitk: sanitize 'open' arguments: command pipeline
gitk: collect construction of blameargs into a single conditional
gitk: sanitize 'open' arguments: simple commands, readable and writable
gitk: sanitize 'open' arguments: simple commands with redirections
gitk: sanitize 'open' arguments: simple commands
gitk: sanitize 'exec' arguments: redirect to process
gitk: sanitize 'exec' arguments: redirections and background
gitk: sanitize 'exec' arguments: redirections
gitk: sanitize 'exec' arguments: 'eval exec'
gitk: sanitize 'exec' arguments: simple cases
gitk: have callers of diffcmd supply pipe symbol when necessary
gitk: treat file names beginning with "|" as relative paths
...
bundle-uri: fix arbitrary file writes via parameter injection
We fetch bundle URIs via `download_https_uri_to_file()`. The logic to
fetch those bundles is not handled in-process, but we instead use a
separate git-remote-https(1) process that performs the fetch for us. The
information about which file should be downloaded and where that file
should be put gets communicated via stdin of that process via a "get"
request. This "get" request has the form "get $uri $file\n\n". As may be
obvious to the reader, this will cause git-remote-https(1) to download
the URI "$uri" and put it into "$file".
The fact that we are using plain spaces and newlines as separators for
the request arguments means that we have to be extra careful with the
respective vaules of these arguments:
- If "$uri" contained a space we would interpret this as both URI and
target location.
- If either "$uri" or "$file" contained a newline we would interpret
this as a new command.
But we neither quote the arguments such that any characters with special
meaning would be escaped, nor do we verify that none of these special
characters are contained.
If either the URI or file contains a newline character, we are open to
protocol injection attacks. Likewise, if the URI itself contains a
space, then an attacker-controlled URI can lead to partially-controlled
file writes.
Note that the attacker-controlled URIs do not permit completely
arbitrary file writes, but instead allows an attacker to control the
path in which we will write a temporary (e.g., "tmp_uri_XXXXXX")
file.
The result is twofold:
- By adding a space in "$uri" we can control where exactly a file will
be written to, including out-of-repository writes. The final
location is not completely arbitrary, as the injected string will be
concatenated with the original "$file" path. Furthermore, the name
of the bundle will be "tmp_uri_XXXXXX", further restricting what an
adversary would be able to write.
Also note that is not possible for the URI to contain a newline
because we end up in `credential_from_url_1()` before we try to
issue any requests using that URI. As such, it is not possible to
inject arbitrary commands via the URI.
- By adding a newline to "$file" we can inject arbitrary commands.
This gives us full control over where a specific file will be
written to. Potential attack vectors would be to overwrite hooks,
but if an adversary were to guess where the user's home directory is
located they might also easily write e.g. a "~/.profile" file and
thus cause arbitrary code execution.
This injection can only become possible when the adversary has full
control over the target path where a bundle will be downloaded to.
While this feels unlikely, it is possible to control this path when
users perform a recursive clone with a ".gitmodules" file that is
controlled by the adversary.
Luckily though, the use of bundle URIs is not enabled by default in Git
clients (yet): they have to be enabled by setting the `bundle.heuristic`
config key explicitly. As such, the blast radius of this parameter
injection should overall be quite contained.
Fix the issue by rejecting spaces in the URI and newlines in both the
URI and the file. As explained, it shouldn't be required to also
restrict the use of newlines in the URI, as we would eventually die
anyway in `credential_from_url_1()`. But given that we're only one small
step away from arbitrary code execution, let's rather be safe and
restrict newlines in URIs, as well.
Eventually we should probably refactor the way that Git talks with the
git-remote-https(1) subprocess so that it is less fragile. Until then,
these two restrictions should plug the issue.
Reported-by: David Leadbeater <dgl@dgl.cx> Based-on-patch-by: David Leadbeater <dgl@dgl.cx> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Justin Tobler [Tue, 20 May 2025 02:26:04 +0000 (21:26 -0500)]
config: quote values containing CR character
When reading the config, values that contain a trailing CRLF are
stripped. If the value itself has a trailing CR, the normal LF that
follows results in the CR being unintentionally stripped. This may lead
to unintended behavior due to the config value written being different
when it gets read.
One such issue involves a repository with a submodule path containing a
trailing CR. When the submodule gets initialized, the submodule is
cloned without being checked out and has "core.worktree" set to the
submodule path. The git-checkout(1) that gets spawned later reads the
"core.worktree" config value, but without the trailing CR, and
consequently attempts to checkout to a different path than intended.
If the repository contains a matching path that is a symlink, it is
possible for the submodule repository to be checked out in arbitrary
locations. This is extra bad when the symlink points to the submodule
hooks directory and the submodule repository contains an executable
"post-checkout" hook. Once the submodule repository checkout completes,
the "post-checkout" hook immediately executes.
To prevent mismatched config state due to misinterpreting a trailing CR,
wrap config values containing CR in double quotes when writing the
entry. This ensures a trailing CR is always separated for an LF and thus
prevented from getting stripped.
Note that this problem cannot be addressed by just quoting each CR with
"\r". The reading side of the config interprets only a few backslash
escapes, and "\r" is not among them. This fix is sufficient though
because it only affects the CR at the end of a line and any literal CR
in the interior is already preserved.
Co-authored-by: David Leadbeater <dgl@dgl.cx> Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Tue, 20 May 2025 06:56:09 +0000 (08:56 +0200)]
Merge branch 'js/fix-open-exec'
This addresses CVE-2025-46835, Git GUI can create and overwrite a
user's files:
When a user clones an untrusted repository and is tricked into editing
a file located in a maliciously named directory in the repository, then
Git GUI can create and overwrite files for which the user has write
permission.
Johannes Sixt [Tue, 20 May 2025 06:54:24 +0000 (08:54 +0200)]
Merge branch 'ml/replace-auto-execok'
This addresses CVE-2025-46334, Git GUI malicious command injection on
Windows.
A malicious repository can ship versions of sh.exe or typical textconv
filter programs such as astextplain. Due to the unfortunate design of
Tcl on Windows, the search path when looking for an executable always
includes the current directory. The mentioned programs are invoked when
the user selects "Git Bash" or "Browse Files" from the menu.
Johannes Sixt [Wed, 14 May 2025 17:56:27 +0000 (19:56 +0200)]
Merge branch 'js/fix-open-exec'
This addresses CVE-2025-27613, Gitk can create and truncate a user's
files:
When a user clones an untrusted repository and runs gitk without
additional command arguments, files for which the user has write
permission can be created and truncated. The option "Support per-file
encoding" must have been enabled before in Gitk's Preferences. This
option is disabled by default.
The same happens when "Show origin of this line" is used in the main
window (regardless of whether "Support per-file encoding" is enabled or
not).
Johannes Sixt [Wed, 14 May 2025 16:27:05 +0000 (18:27 +0200)]
Merge branch 'ah/fix-open-with-stdin'
This addresses CVE-2025-27614, Arbitrary command execution with Gitk:
A Git repository can be crafted in such a way that with some social
engineering a user who has cloned the repository can be tricked into
running any script (e.g., Bourne shell, Perl, Python, ...) supplied by
the attacker by invoking `gitk filename`, where `filename` has a
particular structure. The script is run with the privileges of the user.
Johannes Sixt [Sun, 4 May 2025 19:59:19 +0000 (21:59 +0200)]
git-gui: do not mistake command arguments as redirection operators
Tcl 'open' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are many calls of the
'open' variant that runs a process which construct arguments that are
taken from the Git repository or are user input. However, when file
names or ref names are taken from the repository, it is possible to
find names that have these special forms. They must not be interpreted
by 'open' lest it redirects input or output, or attempts to build a
pipeline using a command name controlled by the repository.
Use the helper function make_arglist_safe, which identifies such
arguments and prepends "./" to force such a name to be regarded as a
relative file name.
After this change the following 'open' calls that start a process do not
apply the argument processing:
In all cases, the command arguments are constant strings (or begin with
a constant string) that are of a form that would not be affected by the
processing anyway.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 18:26:11 +0000 (20:26 +0200)]
git-gui: introduce function git_redir for git calls with redirections
Proc git invokes git and collects all output, which is it returns.
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. A few invocations also pass redirection operators as
command arguments deliberately. Rewrite these cases to use a new
function git_redir that takes two lists, one for the regular command
arguments and one for the redirection operations.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 13:39:03 +0000 (15:39 +0200)]
git-gui: pass redirections as separate argument to git_read
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. Rewrite direct call sites of git_read
to pass intentional redirections as a second (optional) argument.
git_read defers to safe_open_command, but we cannot make it safe, yet,
because one of the callers of git_read is proc git, which does not yet
know which of its arguments are redirections. This is the topic of the
next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 13:06:11 +0000 (15:06 +0200)]
git-gui: pass redirections as separate argument to _open_stdout_stderr
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. Rewrite direct callers of
_open_stdout_stderr to pass intentional redirections as a second
(optional) argument.
Passing arbitrary arguments is not safe right now, but we rename it
to safe_open_command anyway to avoid having to touch the call sites
again later when we make it actually safe.
We cannot make the function safe right away because one caller is
git_read, which does not yet know which of its arguments are
redirections. This is the topic of the next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:24:48 +0000 (13:24 +0200)]
git-gui: convert git_read*, git_write to be non-variadic
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. As a preparation, convert git_read,
git_read_nice, and git_write to take just a single argument that is
the command in a list. Adjust all call sites accordingly.
In the future, this argument will be the regular command arguments and
a second argument will be the redirection operations.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:58:20 +0000 (10:58 -0400)]
git-gui: override exec and open only on Windows
Since aae9560a355d (Work around Tcl's default `PATH` lookup,
2022-11-23), git-gui overrides exec and open on all platforms. But,
this was done in response to Tcl adding elements to $PATH on Windows,
while exec, open, and auto_execok honor $PATH as given on all other
platforms.
Let's do the override only on Windows, restoring others to using their
native exec and open. These honor the sanitized $PATH as that is written
out to env(PATH) in a previous commit. auto_execok is also safe on these
platforms, so can be used for _which.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
The previous commits bb5cb23daf75 (gitk: prevent overly long command
lines, 2023-01-24) rewrote a set of the 'open' calls substantially.
These were then later updated by 7dd272eca153 (gitk: escape file paths
before piping to git log, 2023-01-24) and d5d1b91e5327 (gitk: encode
arguments correctly with "open", 2025-03-07). In the preceding merge,
the conversions to a safe_open variant were undone to ensure that the
principal operation of the new 'open' calls is not modified by accident.
Since the 'open' calls now pass a redirection from a Tcl string as
stdin, convert the calls to 'safe_open_command_redirect'.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 17:21:53 +0000 (19:21 +0200)]
git-gui: use git_read in githook_read
0730a5a3a5e6 ("git-gui - use git-hook, honor core.hooksPath", 2023-09-17)
rewrote githook_read to use `git hook` to run a hook script. The code
that was replaced discovered the hook script file manually and invoked
it using function _open_stdout_stderr. After the rewrite, this function
is still invoked, but it calls into `git` instead of the hook scripts.
Notice though, that we have function git_read that invokes git and
prepares a pipe for the caller to read from. Replace the implementation
of githook_read to be just a wrapper around git_read. This unifies the
way in which the git executable is invoked. git_read ultimately also
calls into _open_stdout_stderr, but it modifies the path to the git
executable before doing so.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:47:04 +0000 (10:47 -0400)]
git-gui: sanitize $PATH on all platforms
Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui removes empty elements from $PATH, and a prior commit made this
remove all non-absolute elements from $PATH. But, this happens only on
Windows. Unsafe $PATH elements in $PATH are possible on all platforms.
Let's sanitize $PATH on all platforms to have consistent behavior. If a
user really wants the current repository on $PATH, they can add its
absolute name to $PATH.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:11:21 +0000 (13:11 +0200)]
git-gui: break out a separate function git_read_nice
There are two callers of git_read that request special treatment using
option --nice. Rewrite them to call a new function git_read_nice that
does the special treatment. Now we can remove all option treatment from
git_read.
git_write has the same capability, but there are no callers that
request --nice. Remove the feature without substitution.
This is a preparation for a later change where we want to make git_read
and friends non-variadic. Then it cannot have optional arguments.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:08:52 +0000 (10:08 -0400)]
git-gui: assure PATH has only absolute elements.
Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui excises all empty paths from $PATH, but still allows '.' or
other relative paths, which can also allow executing code from the
repository. Let's remove anything except absolute elements. While here,
let's remove duplicated elements, which are very common on Windows:
only the first such item can do anything except waste time repeating a
search.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 09:52:35 +0000 (11:52 +0200)]
git-gui: remove option --stderr from git_read
Some callers of git_read want to redirect stderr of the invoked command
to stdout. The function offers option --stderr for this purpose.
However, the option only appends 2>@1 to the commands. The callers can
do that themselves. In lib/console.tcl we even have a caller that
already knew implictly what --stderr does behind the scenes.
This is a preparation for a later change where we want to make git_read
non-variadic. Then it cannot have optional leading arguments.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Mon, 7 Apr 2025 21:12:56 +0000 (17:12 -0400)]
git-gui: cleanup git-bash menu item
git-gui on Git for Windows creates a menu item to start a git-bash
session for the current repository. This menu-item works as desired when
git-gui is installed in the Git for Windows (g4w) distribution, but
not when run from a different location such as normally done in
development. The reason is that git-bash's location is known to be
'/git-bash' in the Unix pathname space known to MSYS, but this is not
known in the Windows pathname space. Instead, git-gui derives a pathname
for git-bash assuming it is at a known relative location.
If git-gui is run from a different directory than assumed in g4w, the
relative location changes, and git-gui resorts to running a generic bash
login session in a Windows console.
But, the MSYS system underlying Git for Windows includes the 'cygpath'
utility to convert between Unix and Windows pathnames. Let's use this so
git-bash's Windows pathname is determined directly from /git-bash.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 26 Apr 2025 16:46:06 +0000 (18:46 +0200)]
git-gui: sanitize 'exec' arguments: background
As in the previous commits, introduce a function that sanitizes
arguments intended for the process, but runs the process in the
background. Convert 'exec' calls to use this new function.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Thu, 3 Apr 2025 04:37:08 +0000 (00:37 -0400)]
git-gui: avoid auto_execok in do_windows_shortcut
git-gui on Windows uses auto_execok to locate git-gui.exe,
which performs the same flawed search as does the builtin exec.
Use _which instead, performing a safe PATH lookup.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 21 Apr 2025 16:14:54 +0000 (18:14 +0200)]
git-gui: sanitize 'exec' arguments: simple cases
Tcl 'exec' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are a number of
invocations of 'exec' which construct arguments that are taken from the
Git repository or a user input. However, when file names or ref names
are taken from the repository, it is possible to find names that have
these special forms. They must not be interpreted by 'exec' lest it
redirects input or output, or attempts to build a pipeline using a
command name controlled by the repository.
Introduce a helper function that identifies such arguments and prepends
"./" to force such a name to be regarded as a relative file name.
Convert those 'exec' calls where the arguments can simply be packed
into a list.
Note that most commands containing the word 'exec' route through
console::exec or console::chain, which we will treat in another commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 2 Apr 2025 21:37:27 +0000 (17:37 -0400)]
git-gui: avoid auto_execok for git-bash menu item
On Windows, git-gui offers to open a git-bash session for the current
repository from the menu, but uses [auto_execok start] to get the
command to actually run that shell.
The code for auto_execok, in /usr/share/tcl8.6/tcl.init, has 'start' in
the 'shellBuiltins' list for cmd.exe on Windows: as a result,
auto_execok does not actually search for start, meaning this usage is
technically ok with auto_execok now. However, leaving this use of
auto_execok in place will just induce confusion about why a known unsafe
function is being used on Windows. Instead, let's switch to using our
known safe _which function that looks only in $PATH, excluding the
current working directory.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 21 Apr 2025 15:07:10 +0000 (17:07 +0200)]
git-gui: treat file names beginning with "|" as relative paths
The Tcl 'open' function has a very wide interface. It can open files as
well as pipes to external processes. The difference is made only by the
first character of the file name: if it is "|", a process is spawned.
We have a number of calls of Tcl 'open' that take a file name from the
environment in which Git GUI is running. Be prepared that insane values
are injected. In particular, when we intend to open a file, do not take
a file name that happens to begin with "|" as a request to run a process.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 4 Apr 2025 20:55:59 +0000 (16:55 -0400)]
git-gui: remove unused proc is_shellscript
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added is_shellscript to test if a file
is executable by the shell, used only when searching for textconv
filters. The previous commit rearranged the tests for finding such
filters, and removed the only user of is_shellscript. Remove this
function.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:37:35 +0000 (13:37 +0200)]
git-gui: remove git config --list handling for git < 1.5.3
git-gui uses `git config --null --list` to parse configuration. Git
versions prior to 1.5.3 do not have --null and need different treatment.
Nobody should be using such an old version anymore. (Moreover, since 0730a5a3a, git-gui requires git v2.36 or later). Keep only the code for
modern Git.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 18 May 2025 14:08:06 +0000 (16:08 +0200)]
git-gui: remove special treatment of Windows from open_cmd_pipe
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added open_cmd_pipe to run text
conversion in support of blame, with special handling for shell
scripts on Windows. To determine whether the command is a shell
script, 'lindex' is used to pick off the first token from the command.
However, cmd is actually a command string taken from .gitconfig
literally and is not necessarily a syntactically correct Tcl list.
Hence, it cannot be processed by 'lindex' and 'lrange' reliably.
Pass the command string to the shell just like on non-Windows
platforms to avoid the potentially incorrect treatment.
A use of 'auto_execok' is removed by this change. This function is
dangerous on Windows, because it searches programs in the current
directory. Delegating the path lookup to the shell is safe, because
/bin/sh and /bin/bash follow POSIX on all platforms, including the
Git for Windows port.
A possible regression is that the old code, given filter command of
'foo', could find 'foo.bat' as a script, and not just bare 'foo', or
'foo.exe'. This rewrite requires explicitly giving the suffix if it is
not .exe.
This part of Git GUI can be exercised using
git gui blame -- some.file
while some.file has a textconv filter configured and has unstaged
modifications.
Helped-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 2 May 2025 15:39:55 +0000 (11:39 -0400)]
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui provides an implementation to detach HEAD on Git versions prior
to 1.5.3. Nobody should be using such an old version anymore.
(Moreover, since 0730a5a3a, git-gui requires git v2.36 or later).
Keep only the code for modern Git.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
[j6t: message tweaked] Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Sun, 6 Apr 2025 22:20:14 +0000 (18:20 -0400)]
git-gui: use only the configured shell
git-gui has a few places where a bare "sh" is passed to exec, meaning
that the first instance of "sh" on $PATH will be used rather than the
shell configured. This violates expectations that the configured shell
is being used. Let's use [shellpath] everywhere.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 20 Sep 2023 21:56:14 +0000 (17:56 -0400)]
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
Since b792230 ("git-gui: Show a progress meter for checking out files",
2007-07-08), git-gui includes a workaround for Tcl that does not support
using 2>@1 to redirect stderr to stdout. Tcl added such support in
8.4.7, released in 2004, and this is fully supported in all 8.5
releases.
As git-gui has a hard-coded requirement for Tcl >= 8.5, the workaround
is no longer needed. Delete it.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Tue, 1 Apr 2025 15:45:06 +0000 (11:45 -0400)]
git-gui: make _shellpath usable on startup
Since commit d5257fb3c1de (git-gui: handle textconv filter on
Windows and in development, 2010-08-07), git-gui will search for a
usable shell if _shellpath is not configured, and on Windows may
resort to using auto_execok to find 'sh'. While this was intended for
development use, checks are insufficient to assure a proper
configuration when deployed where _shellpath is always set, but might
not give a usable shell.
Let's make this more robust by only searching if _shellpath was not
defined, and then using only our restricted search functions.
Furthermore, we should convert to a Windows path on Windows. Always
check for a valid shell on startup, meaning an absolute path to an
executable, aborting if these conditions are not met.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 2 Apr 2025 15:23:03 +0000 (11:23 -0400)]
git-gui: use [is_Windows], not bad _shellpath
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added open_cmd_pipe, with special
handling for Windows detected by seeing that _shellpath does not
point to an executable shell. That is bad practice, and is broken by
the next commit that assures _shellpath is valid on all platforms.
Fix this by using [is_Windows] as done for all Windows specific code.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Thu, 3 Apr 2025 14:26:21 +0000 (10:26 -0400)]
git-gui: _which, only add .exe suffix if not present
The _which function finds executables on $PATH, and adds .exe on Windows
unless -script was given. However, win32.tcl executes "wscript.exe"
and "cscript.exe", both of which fail as _which adds .exe to both. This
is already fixed in git-gui released by Git for Windows. Do so here.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Taylor Blau [Fri, 23 May 2025 21:04:21 +0000 (17:04 -0400)]
Merge branch 'js/fix-open-exec-2.40.0' into js/fix-open-exec
Branch js/fix-open-exec-2.40.0 converts `open` and `exec` calls to call
wrappers that sanitze the command arguments. This side branch updates
three `open` calls that are in conflict with the fix in the preceding
commit. To keep the intended operation of the 'open' calls, this merge
does not try to merge and resolve the conflicts, but ignores the
conversions that are brought in by the side branch, taking "ours" side
of the code in these three cases.
New fixes are the topic of the next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
While "exec" uses a normal arguments list which is applied as
command + arguments (and redirections, etc), "open" uses a single
argument which is this command+arguments, where the command and
arguments are a list inside this one argument to "open".
Commit bb5cb23 (gitk: prevent overly long command lines 2023-05-08)
changed several values from individual arguments in that list (hashes
and file names), to a single value which is fed to git via redirection
to its stdin using "open" [1].
However, it didn't ensure correctly that this aggregate value in this
string is interpreted as a single element in this command+args list.
It did just enough so that newlines (which is how these elements are
concatenated) don't split this single list element.
A followup commit at the same patchset: 7dd272e (gitk: escape file
paths before piping to git log 2023-05-08) added a bit more, by
escaping backslahes and spaces at the file names, so that at least
it doesn't break when such file names get used there.
But these are not enough. At the very least tab is missing, and more,
and trying to manually escape every possible thing which can affect
how this string is interpreted in a list is a sub-par approach.
The solution is simply to tell tcl "this is a single list element".
which we can do by aggregating this value completely normally (hashes
and files separated by newlines), and then do [list $value].
So this is what this commit does, for all 3 places where bb5cb23
changed individual elements into an aggregate value.
[1]
That was not a fully accurate description. The accurate version
is that this string originally included two lists: hashes and files.
When used with "open" these lists correctly become the individual
elements of these lists, even if they contain spaces etc, so the
arguments which were used at this "git" commands were correct.
Commit bb5cb23 couldn't use these two lists as-is, because it needed
to process the individual elements in them (one element per line of
the aggregate value), and the issue is that ensuring this aggregate
is indeed interpreted as a single list element was sub-par.
Note: all the (double) quotes before/after the modification are not
required and with zero effect, even for \n. But this commit preserves
the original quoting form intentionally. It can be cleaned up later.
Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 23 Mar 2025 21:34:11 +0000 (22:34 +0100)]
gitk: collect construction of blameargs into a single conditional
The command line to invoke 'git blame' for a single line is constructed
using several if-conditionals, each with the same condition
{$from_index new {}}. Merge all of them into a single conditional.
This requires to duplicate significant parts of the command, but it
helps the next change, where we will have to deal with a nested list
structure.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Thu, 20 Mar 2025 19:00:57 +0000 (20:00 +0100)]
gitk: sanitize 'open' arguments: simple commands with redirections
As in the previous commits, introduce a function that sanitizes
arguments intended for the process and in addition allows to pass
redirections, which are passed to Tcl's 'open' verbatim.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Thu, 20 Mar 2025 18:32:56 +0000 (19:32 +0100)]
gitk: sanitize 'open' arguments: simple commands
Tcl 'open' treats the second argument as a command when it begins
with |. The remainder of the argument is a list comprising the command
and its arguments. It assigns special meaning to these arguments when
they begin with a redirection, pipe or background operator. There are a
number of invocations of 'open' which construct arguments that are
taken from the Git repository or a user input. However, when file names
or ref names are taken from the repository, it is possible to find
names which have these special forms. They must not be interpreted by
'open' lest it redirects input or output, or attempts to build a
pipeline using a command name controlled by the repository.
Introduce a helper function that identifies such arguments and prepends
"./" to force such a name to be regarded as a relative file name.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 29 Mar 2025 16:35:19 +0000 (17:35 +0100)]
gitk: sanitize 'exec' arguments: redirect to process
Convert one 'exec' call that sends output to a process (pipeline).
Fortunately, the command does not contain any variables. For this
reason, just treat it as a "redirection".
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 29 Mar 2025 16:21:27 +0000 (17:21 +0100)]
gitk: sanitize 'exec' arguments: redirections and background
Convert 'exec' calls that both redirect output to a file and run the
process in the background. 'safe_exec_redirect' can take both these
"redirections" in the second argument simultaneously.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 29 Mar 2025 16:01:54 +0000 (17:01 +0100)]
gitk: sanitize 'exec' arguments: redirections
As in the previous commits, introduce a function that sanitizes
arguments intended for the process and in addition allows to pass
redirections verbatim, which are interpreted by Tcl's 'exec'.
Redirections can include the background operator '&'.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 29 Mar 2025 15:51:29 +0000 (16:51 +0100)]
gitk: sanitize 'exec' arguments: 'eval exec'
Convert calls of 'exec' where the arguments are already available in
a list and 'eval' is used to unpack the list. Use 'concat' to unite
the arguments into a single list before passing them to 'safe_exec'.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 17 Mar 2025 21:59:27 +0000 (22:59 +0100)]
gitk: sanitize 'exec' arguments: simple cases
Tcl 'exec' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are a number of
invocations of 'exec' which construct arguments that are taken from the
Git repository or a user input. However, when file names or ref names
are taken from the repository, it is possible to find names with have
these special forms. They must not be interpreted by 'exec' lest it
redirects input or output, or attempts to build a pipeline using a
command name controlled by the repository.
Introduce a helper function that identifies such arguments and prepends
"./" to force such a name to be regarded as a relative file name.
Convert those 'exec' calls where the arguments can simply be packed
into a list.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 17 Mar 2025 20:39:58 +0000 (21:39 +0100)]
gitk: have callers of diffcmd supply pipe symbol when necessary
Function 'diffcmd' derives which of git diff-files, git diff-index, or
git diff-tree must be invoked depending on the ids provided. It puts
the pipe symbol as the first element of the returned command list.
Note though that of the four callers only two use the command with
Tcl 'open' and need the pipe symbol. The other two callers pass the
command to Tcl 'exec' and must remove the pipe symbol.
Do not include the pipe symbol in the constructed command list, but let
the call sites decide whether to add it or not. Note that Tcl 'open'
inspects only the first character of the command list, which is also
the first character of the first element in the list. For this reason,
it is valid to just tack on the pipe symbol with |$cmd and it is not
necessary to use [concat | $cmd].
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 17 Mar 2025 19:36:04 +0000 (20:36 +0100)]
gitk: treat file names beginning with "|" as relative paths
The Tcl 'open' function has a vary wide interface. It can open files as
well as pipes to external processes. The difference is made only by the
first character of the file name: if it is "|", an process is spawned.
We have a number of calls of Tcl 'open' that take a file name from the
environment in which Gitk is running. Be prepared that insane values are
injected. In particular, when we intend to open a file, do not mistake
a file name that happens to begin with "|" as a request to run a process.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Junio C Hamano [Mon, 13 Jan 2025 20:55:26 +0000 (12:55 -0800)]
Sync with Git 2.47.2
Git 2.47.2
# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE4fA2sf7nIh/HeOzvsLXohpav5ssFAmdkT1sACgkQsLXohpav
# 5svdhRAAq0WoZIg+33vYNNVSTm3Ux9RJslmXs3lQuhuUJ61hK/28drSLU29GH7x7
# 3nmmjp1cegnXRVLBAfoYDdzPprNNrQFQEHQEzgG/GDZw0OXn+WTZuNyrrUYoa+sd
# QSLlElRj2qrpHIMOsMIBKBSNB+qjJHOMGdxcBAS768TfnQpGIpc1KJa24TxsVBzC
# ScP4uvrFfPyQrqFUgiUhCeqLnO/6T5i/QAn/8cS5a1+zor5ZHSlw28TZTOxN2odo
# Rulp/FtehiDEzmRowgD3M4fImAPY6Ib6VORCYASqpJFFla30tu2bQqEi6raOMTec
# hg5Ibkmj6fHFONaYvoTMRkYHmtUnNgIPU/CYPwswNk8w1+PPQfJ+TYjBXOQgdTLW
# F0azHBHh7NRmEHVydiF9CqjgNVRzjO4IEZfGqXNFPPMvR6UUzDaIkrpYbwXBFMin
# GNPV3QISeXj9ROjJoCv0nclXETwWemykjZlD6b5krXn5TaJlFb+69qJvXrCLq5WY
# EoevSqKkB9HVK9si7P8Sh1cPGOr3kfiFPmMNKFVI8l0+iDFgBywOomWNS/JEzqu1
# nN142DKdL1W/rkeMUhbX2h11CZNvHKIOy3iaA4MTOing8/eMzyUUQ73Ck7odYs4f
# rZ0tTXKJhxojPvBpTxYe9SxM0bDLREiOv0zX76+sIuhbAQCmk0o=
# =MNNf
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 19 Dec 2024 08:52:43 AM PST
# gpg: using RSA key E1F036B1FEE7221FC778ECEFB0B5E88696AFE6CB
# gpg: Good signature from "Junio C Hamano <gitster@pobox.com>" [ultimate]
# gpg: aka "Junio C Hamano <junio@pobox.com>" [ultimate]
# gpg: aka "Junio C Hamano <jch@google.com>" [ultimate]
* tag 'v2.47.2':
Git 2.47.2
Git 2.46.3
Git 2.45.3
Git 2.44.3
Git 2.43.6
Git 2.42.4
Git 2.41.3
Git 2.40.4
credential: disallow Carriage Returns in the protocol by default
credential: sanitize the user prompt
credential_format(): also encode <host>[:<port>]
t7300: work around platform-specific behaviour with long paths on MinGW
compat/regex: fix argument order to calloc(3)
mingw: drop bogus (and unneeded) declaration of `_pgmptr`
ci: remove 'Upload failed tests' directories' step from linux32 jobs
Junio C Hamano [Fri, 10 Jan 2025 17:19:33 +0000 (09:19 -0800)]
Merge branch 'ps/build-sign-compare'
Last-minute fix for a regression in "git blame --abbrev=<length>"
when insane <length> is specified; we used to correctly cap it to
the hash output length but broke it during the cycle.
* ps/build-sign-compare:
builtin/blame: fix out-of-bounds write with blank boundary commits
builtin/blame: fix out-of-bounds read with excessive `--abbrev`
"Why would one want to run it in parallel?" I hear you ask. I am glad
you are curious, because a curious story is what it is, indeed.
The `GIT-VERSION-GEN` script is quite a pillar of Git's source code,
with most lines being unchanged for the past 15 years. Until the v2.48.0
release candidate cycle.
Its original purpose was to generate the version string and store it in
the `GIT-VERSION-FILE`.
This paradigm changed quite dramatically when support for building with
Meson was introduced. Most crucially, a38edab7c88b (Makefile: generate
doc versions via GIT-VERSION-GEN, 2024-12-06) changed the way the
documentation is built by using the `GIT-VERSION-GEN` file to write out
the `asciidocor-extensions.rb` and `asciidoc.conf` files with now
hard-coded version strings.
Crucially, the Makefile rule to generate those files needs to be run in
every build because `GIT_VERSION` could have been specified in the
`make` command-line, which would require these files to be modified.
This introduced a surprising race condition!
And this is how that race surfaces: When calling `make -j2 html man`
from the top-level directory (a variant of which is invoked in Git for
Windows' release process), two sub-processes are spawned, a `make -C
Documentation html` one and a `make -C Documentation man` one. Both run
the rule to (re-)generate `asciidoctor-extensions.rb` or
`asciidoc.conf`, invoking `GIT-VERSION-GEN` to do so. That script first
generates a temporary file (appending the `+` character to the
filename), then looks whether it contains something different than the
already existing file (if it exists, that is), and either replaces it if
needed, or removes the temporary file. If one of the two parallel
invocations removes that temporary file before the other can compare it,
or even worse: if one tries to replace the target file just after the
other _started_ writing the temporary file (but did not finish writing
it yet), that race condition now causes bad builds.
This may sound highly theoretical, but due to the design of Git's build
process, Git for Windows is forced to use a (slow) POSIX emulation layer
to run that script and in the blink of an eye it becomes very much not
theoretical at all. See Exhibit A: These GitHub workflow runs failed
because one of the two competing `make` processes tried to remove the
temporary file when the other process had already done so:
While it is undesirable to run this script over and over again,
certainly when this involves above-mentioned slow POSIX emulation layer,
the stage of the release cycle in which we are presently finding
ourselves does not lend itself to a re-design where this script could be
run once, and once only, but instead dictates that a quick and reliable
work-around be implemented that prevents the race condition without
changing the overall architecture of the build process.
This patch does that: By using a filename suffix for the temporary file
which is based on the currently-executing script's process ID, We
guarantee that the two competing invocations cannot overwrite or remove
each others' temporary files.
The filename suffix still ends in `+` to ensure that the temporary
artifacts are matched by the `*+` pattern in `.gitignore` that was added
in f9bbaa384ef (Add intermediate build products to .gitignore,
2009-11-08).
Helped-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/blame: fix out-of-bounds write with blank boundary commits
When passing the `-b` flag to git-blame(1), then any blamed boundary
commits which were marked as uninteresting will not get their actual
commit ID printed, but will instead be replaced by a couple of spaces.
The flag can lead to an out-of-bounds write as though when combined with
`--abbrev=` when the abbreviation length is longer than `GIT_MAX_HEXSZ`
as we simply use memset(3p) on that array with the user-provided length
directly. The result is most likely that we segfault.
An obvious fix would be to cull `length` to `GIT_MAX_HEXSZ` many bytes.
But when the underlying object ID is SHA1, and if the abbreviated length
exceeds the SHA1 length, it would cause us to print more bytes than
desired, and the result would be misaligned.
Instead, fix the bug by computing the length via strlen(3p). This makes
us write as many bytes as the formatted object ID requires and thus
effectively limits the length of what we may end up printing to the
length of its hash. If `--abbrev=` asks us to abbreviate to something
shorter than the full length of the underlying hash function it would be
handled by the call to printf(3p) correctly.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/blame: fix out-of-bounds read with excessive `--abbrev`
In 6411a0a896 (builtin/blame: fix type of `length` variable when
emitting object ID, 2024-12-06) we have fixed the type of the `length`
variable. In order to avoid a cast from `size_t` to `int` in the call to
printf(3p) with the "%.*s" formatter we have converted the code to
instead use fwrite(3p), which accepts the length as a `size_t`.
It was reported though that this makes us read over the end of the OID
array when the provided `--abbrev=` length exceeds the length of the
object ID. This is because fwrite(3p) of course doesn't stop when it
sees a NUL byte, whereas printf(3p) does.
Fix the bug by reverting back to printf(3p) and culling the provided
length to `GIT_MAX_HEXSZ` to keep it from overflowing when cast to an
`int`.
Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t-reftable-basics: allow for `malloc` to be `#define`d
As indicated by the `#undef malloc` line in `reftable/basics.h`, it is
quite common to use allocators other than the default one by defining
`malloc` constants and friends.
This pattern is used e.g. in Git for Windows, which uses the powerful
and performant `mimalloc` allocator.
Furthermore, in `reftable/basics.c` this `#undef malloc` is
_specifically_ disabled by virtue of defining the
`REFTABLE_ALLOW_BANNED_ALLOCATORS` constant before including
`reftable/basic.h`, to ensure that such a custom allocator is also used
in the reftable code.
However, in 8db127d43f5b (reftable: avoid leaks on realloc error,
2024-12-28) and in 2cca185e8517 (reftable: fix allocation count on
realloc error, 2024-12-28), `reftable_set_alloc()` function calls were
introduced that pass `malloc`, `realloc` and `free` function pointers as
parameters _after_ `reftable/basics.h` ensured that they were no longer
`#define`d. This would override the custom allocator and re-set it to
the default allocator provided by, say, libc or MSVCRT.
This causes problems because those calls happen after the initial
allocator has already been used to initialize an array, which is
subsequently resized using the overridden default `realloc()` allocator.
You cannot mix and match allocators like that, which leads to a
`STATUS_HEAP_CORRUPTION` (C0000374) on Windows, and when running this
unit test through shell and/or `prove` (which only support 7-bit status
codes), it surfaces as exit code 127.
It is actually unnecessary to use those function pointers to
`malloc`/`realloc`/`free`, though: The `reftable` code goes out of its
way to fall back to the initial allocator when passing `NULL` parameters
instead. So let's do that instead of causing heap corruptions.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sam James [Wed, 8 Jan 2025 03:42:37 +0000 (03:42 +0000)]
meson: fix perl dependencies
`generate_perl_command` needs `depends: [git_version_file]` and the uses
in top-level meson.build were fine, but the ones in perl/ weren't, causing
parallel build failures in some cases as GIT-BUILD-OPTIONS wasn't yet
available.
Signed-off-by: Sam James <sam@gentoo.org> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Correct verb tense, add missing words, avoid double blank lines,
and rephrase things that don’t read well to me like “Turn this linkage
to relative paths”.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
9e2b7005be (fetch set_head: add warn-if-not-$branch option, 2024-12-05)
tried to expand the advice message for set_head with the new option, but
unfortunately did not manage to add the right incantation. Fix the
advice message with the correct usage of warn-if-not-$branch.
Reported-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
