Skip to content

Commit

Permalink
fix: More robustly ensure "installation" config is not local
Browse files Browse the repository at this point in the history
When invoking `git` to find the configuration file path associated
with the `git` installation itself, this sets `GIT_DIR` to a path
that cannot be a `.git` directory for any repository, to keep
`git config -l` from including any local scope entries in the
output of the `git config -l ...` command that is used to find the
origin for the first Git configuration variable.

Specifically, a path to the null device is used. This is
`/dev/null` on Unix and `NUL` on Windows. This is not a directory,
and when treated as a file it is always treated as empty: reading
from it, if successful, reaches end-of-file immediately.

This problem is unlikely since #1523, which caused this `git`
invocation to use a `/tmp`-like location (varying by system and
environment) as its current working directory. Although the goal of
that change was just to improve performance, it pretty much fixed
the bug where local-scope configuration could be treated as
installation-level configuration when no configuration variables
are available from higher scopes.

This change further hardens against two edge cases:

- If the `git` command is an old and unpatched vulnerable version
  in which `safe.directory` is not yet implemented, or in which
  GHSA-j342-m5hw-rr3v
  or other vulnerabilities where `git` would perform operations on
  untrusted local repositories owned by other users are unpatched,
  then a `.git` subdirectory of a shared `/tmp` or `/tmp`-like
  directory could be created by another account, and its local
  configuration would still have been used. (This is not a bug in
  gitoxide per se; having vulnerable software installed that other
  software may use is inherently insecure. But it is nice to offer
  a small amount of protection against this when readily feasible.)

- If the `/tmp`-like location is a Git repository owned by the
  current user, then its local configuration would have been used.

Any path guaranteed to point to a nonexistent entry or one that is
guaranteed to be (or to be treated as) an empty file or directory
should be sufficient here. Using the null device, even though it is
not directory-like, seems like a reasonably intuitive way to do it.

A note for Windows: There is more than one reasonable path to the
null device. One is DOS-style relative path `NUL`, as used here.
One of the others, which `NUL` in effect resolves to when opened,
is the fully qualified Windows device namespace path `\\.\NUL`. I
used the former here to ensure we avoid any situation where `git`
would misinterpret a `\` in `\\.\NUL` in a POSIX-like fashion. This
seems unlikely, and it could be looked into further if reasons
surface to prefer `\\.\NUL`.

One possible reason to prefer `\\.\NUL` is that which names are
treated as reserved legacy DOS device names changes from version to
version of Windows, with Windows 11 treating some of them as
ordinary filenames. However, while this affects names such as
`CON`, it does not affect `NUL`, at least written unsuffixed. I'm
not sure if any Microsoft documentation has yet been updated to
explain this in detail, but see:

- dotnet/docs#41193
- python/cpython#95486 (comment)
- python/cpython#95486 (comment)

At least historically, it has been possible on Windows, though
rare, for the null device to be absent. This was the case on
Windows Fundamentals for Legacy PCs (WinFPE). Even if that somehow
were ever to happen today, this usage should be okay, because
attempting to open the device would still fail rather than open
some other file (as may even be happening in Git for Windows
already), the name `NUL` would still presumably be reserved (much
as the names `COM?` where `?` is replaced with a Unicode
superscript 1, 2, or 3 are reserved even though those devices don't
really exist), and I think `git config -l` commands should still
shrug off the error opening the file and give non-local-scope
configuration, as it does when `GIT_DIR` is set to a nonexistent
location.
  • Loading branch information
EliahKagan committed Aug 28, 2024
1 parent 15cec4e commit 7280a2d
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions gix-path/src/env/git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ pub(super) static EXE_NAME: &str = "git";
/// The git executable is the one found in PATH or an alternative location.
pub(super) static EXE_INFO: Lazy<Option<BString>> = Lazy::new(exe_info);

#[cfg(windows)]
static NULL_DEVICE: &str = "NUL";
#[cfg(not(windows))]
static NULL_DEVICE: &str = "/dev/null";

fn exe_info() -> Option<BString> {
let mut cmd = git_cmd(EXE_NAME.into());
gix_trace::debug!(cmd = ?cmd, "invoking git for installation config path");
Expand Down Expand Up @@ -113,6 +118,7 @@ fn git_cmd(executable: PathBuf) -> Command {
// git 2.8.0 and higher support --show-origin.
cmd.args(["config", "-lz", "--show-origin", "--name-only"])
.current_dir(env::temp_dir())
.env("GIT_DIR", NULL_DEVICE)
.stdin(Stdio::null())
.stderr(Stdio::null());
cmd
Expand Down

0 comments on commit 7280a2d

Please sign in to comment.