LWN: Comments on "Git security fixes released"

Git security fixes released

mathstuf — Thu, 14 Apr 2022 17:37:11 +0000

LFS does technically have a flag, but it is an envvar which is tedious to consistently maintain.

And while true, the build system is where I'd expect any surreptitious behavior to go since that already has a blanket contract to execute code.

Git security fixes released

mathstuf — Thu, 14 Apr 2022 17:34:46 +0000

Probably as many that lurk in and around functions passed to signal(7) APIs.

Git security fixes released

agateau — Thu, 14 Apr 2022 16:00:35 +0000

More details can be found in this article: https://blog.sonarsource.com/securing-developer-tools-git-integrations.

Git security fixes released

marcH — Thu, 14 Apr 2022 15:18:45 +0000

> (MSVC still has problems with some long filenames)

_This_ is the mind-boggling part.

> which made projects run back to `C:\foo` and lose their spaces-in-paths test cases.

I wonder how many security holes lie there...

Git security fixes released

ericproberts — Thu, 14 Apr 2022 13:12:25 +0000

This type of vulnerability feels like it should be more common, since a lot of tools walk back directories.

For instance (on OSX but I think it would work the same on Linux):

$ touch -p /tmp/node_modules/lodash
$ echo 'console.log("pwn")' >/tmp/node_modules/lodash/index.js
$ sudo chown -R nobody:nobody /tmp/node_modules

$ mkdir /tmp/nodeproject && cd /npm/nodeproject
$ npm init && npm install
$ node <<<'require("lodash")'
pwn

This requires importing an uninstalled package, so it seems of limited usefulness, but some packages will use a try { require("...") } catch {} for optional dependencies.

Git security fixes released

nybble41 — Thu, 14 Apr 2022 12:44:10 +0000

> I enable LFS per-repo because I don't want to clone a repo and get smacked with tons of data I don't care about because I'm fixing a typo.

LFS could have an option to enable or disable it which is separate from the definitions of the filter commands. Only the filter command lines would need to be set globally.

> At $DAYJOB, we also use repo-local aliases which call scripts in the repo so that we can maintain them sensibly.

That's obviously a gaping security hole if you might run Git commands from a repo someone else has write access to, whether deliberately or accidentally.

Perhaps `git config` could add the ability to configure per-repo settings in the global user configuration files, which you could then import explicitly? Or as with the fix in this article you could globally configure a list of "trusted" repos (or paths containing repos) which are allowed to define commands in the repos' configuration files.

Git security fixes released

hmh — Thu, 14 Apr 2022 11:27:06 +0000

Apparently, there is a fresh new release to address some of it, adding the equivalent to a disable switch...

https://public-inbox.org/git/xmqq1qy04iqa.fsf@gitster.g/

However, one should keep in mind that wherever the security fix broke a build system under "fakeroot" or "sudo", that build system is running git as a privileged (or fake-privileged) user in a generally unsafe way. It might not be a big deal in some particular setup, obviously, but it is a very bad, and unsafe, practice in the general case.

IMO, fixing the build system to never call git in targets that are run as a privileged user (install, uninstall, etc) would be the appropriate response, medium-term. Short-term, you do what you need to do, of course... but it would be best to ensure parent directories are secure first thing...

And yeah, that does mean one should rethink how those powerful git-aware prompts or powerlines in shells should be enabled in a path-aware way or not enabled by default...

Git security fixes released

NYKevin — Thu, 14 Apr 2022 02:39:31 +0000

Well, there's also C:\Program Files, so at least your actual binary has to support spaces in its path, but the Python people managed to even avoid doing that by installing into C:\Python3x (where x is the minor version).

Git security fixes released

NYKevin — Thu, 14 Apr 2022 02:36:24 +0000

As someone who has done similar things (but not for Git specifically), I can tell you why people do this: We can't stand waiting for the damn prompt to finish rendering. My prompt should print in under ~200 ms, but that's unrealistic if you're going to invoke half a dozen subshells on random command substitutions and the like. There are basically two ways out of this:

1. The right, but difficult way: Spawn a daemon or daemon-like process (running as yourself, e.g. using systemd's --user functionality, or just by persuading your shell to fork off a coprocess somehow) which knows how to find all of the information that your shell needs to print out its prompt, and then have your shell asynchronously send "I cd'd to a new directory" notifications to this daemon and asynchronously read updated prompt information from the daemon via some pipe/socket nonsense. Then you can get away with running heavy stuff like Git or Mercurial in the daemon, and your shell updates its prompt when the daemon eventually gets back to it.
2. The wrong, but easy way: Take shortcuts such as looking for .git, .hg, etc. by hand, hard-coding paths that you "know" have certain semantics in practice (e.g. "I know that the Git repos always live under /foo/bar/ on this system, so I will match ${PWD} against that prefix and then do [[ -d /foo/bar/$WHATEVER_WE_MATCHED/.git ]] to see whether we're in a Git repo"), etc., and never, ever spawn any more subshells than absolutely necessary (e.g. if a shell function has to return a string, do REPLY="$value" instead of printf "%s\n" "$value", and then you don't need to run it in a command substitution).

Tooling for (1) exists (e.g. zsh-async), but it's slightly more of a PITA to set up compared to just doing (2) everywhere (and also, I don't like running random GitHub code at $WORKPLACE unless it's in the package repository and somebody has vetted it), so you end up with horrible spaghetti code all over your .bashrc/.zshrc/what-have-you. It's a problem, but I wouldn't call it a big problem because ultimately it's just a shell prompt, it doesn't (directly) make $WORKPLACE money or anything like that.

Git security fixes released

hmh — Wed, 13 Apr 2022 22:42:38 +0000

It broke a lot of things related to calling git in the build system while under sudo or fakeroot, yes.

Including in git itself. Look at the reply to the announcement message...

Git security fixes released

mathstuf — Wed, 13 Apr 2022 22:09:00 +0000

> Perhaps configuration options which define external commands such as this should only be honored in the system and user global configuration files, and not the ones in the repos? (This would also include aliases, diff tools, filters, etc.)

Please no. I enable LFS per-repo because I don't want to clone a repo and get smacked with tons of data I don't care about because I'm fixing a typo. Fedora's `git-lfs` package now has a(n opt-in) mechanism to stop it from installing its filter configuration on the system because I asked for it :) .

At $DAYJOB, we also use repo-local aliases which call scripts in the repo so that we can maintain them sensibly.

Git security fixes released

nybble41 — Wed, 13 Apr 2022 21:47:12 +0000

The article refers to `core.fsmonitor`:

> If set, the value of this variable is used as a command which will identify all files that may have changed since the requested date/time. This information is used to speed up git by avoiding unnecessary processing of files that have not changed.

… which seems like a natural fit for a command like `git status`.

Perhaps configuration options which define external commands such as this should only be honored in the system and user global configuration files, and not the ones in the repos? (This would also include aliases, diff tools, filters, etc.)

Git security fixes released

dskoll — Wed, 13 Apr 2022 19:26:02 +0000

Well, I guess it would fail. But so far it works for all of my use-cases. The source, if anyone is interested, is at gitquickbranch.c.

Git security fixes released

dtlin — Wed, 13 Apr 2022 18:35:39 +0000

But does it know how to follow git-worktree to the base repository? There's no .git/HEAD there.

Git security fixes released

dskoll — Wed, 13 Apr 2022 18:21:33 +0000

I use PS1 to set the git branch in my prompt, but I don't actually invoke git to find the branch. I wrote my own program that traverses up to / looking for .git/HEAD. It's way smaller and faster than git and also much simpler.

Git security fixes released

jthill — Wed, 13 Apr 2022 17:26:45 +0000

You can't alias builtins for obviously good reason, I think it's that a planted config could define aliases that will override the user's possibly in a way that's hard to detect, so the user might use `git st` whether in their prompt generator or otherwise. Computer labs full of teenage boys . . . yeah, okay.

The check is only on auto-discovered repos, easy enough to shut it off by specifying the repo in GIT_DIR when you know the one you want.

Seems to me the safe-directory entries should support globbing, making trusted environments hard to administer is legitimately objectionable.

Git security fixes released

mathstuf — Wed, 13 Apr 2022 17:09:12 +0000

Builtins cannot be aliased (AFAIK). I suspect there are hooks one could leave laying around that end up with arbitrary code execution. `pre-auto-gc` seems like something `git status` could trigger.

Git security fixes released

mathstuf — Wed, 13 Apr 2022 17:06:36 +0000

> was so that apps would have to support spaces in paths if they wanted to be capable of moving away from legacy FAT "mydocu~1" altnames.

Which was naive. Instructions just said to go to `C:\foo` instead. This may have worked for a while, but then `Documents and Settings` made it easy to run into path length limits (MSVC still has problems with some long filenames) which made projects run back to `C:\foo` and lose their spaces-in-paths test cases.

> but actual development tools like build systems - in the 2020s that don't support spaces in paths is mind-boggling.

IME, the issues actually usually come from autoconf-based (or raw Makefile) packages where quoting those `$(topdir)` and such expansions is pretty lax. In CMake, you're usually OK, but sometimes someone does something that tries to pass paths around without any thought about "might be seen as two words later".

Git security fixes released

apoelstra — Wed, 13 Apr 2022 16:15:56 +0000

I'm a bit confused -- can `git status` be made to run arbitrary code? Is the issue that somebody could, for example, edit `.git/config` to alias `git status` to something malicious?

Git security fixes released

nye — Wed, 13 Apr 2022 15:40:45 +0000

Thanks, this appears to be the missing link - to my great surprise, it turns out that by default "Authenticated Users" has the "Create folders / append data" bit set explicitly for C: by default.

That's pretty horrifying but I suppose there are compatibility concerns that would make it hard to fix now.

Git security fixes released

ceplm — Wed, 13 Apr 2022 15:00:31 +0000

Other shells (I use fish) have such prompts running scripts as default.

Git security fixes released

Karellen — Wed, 13 Apr 2022 14:26:41 +0000

IIRC one of the reasons that Windows started creating paths with spaces by default back in the '90s (e.g. "My Documents", and later "Documents and Settings") was so that apps would have to support spaces in paths if they wanted to be capable of moving away from legacy FAT "mydocu~1" altnames.

The idea that there are Windows applications - and not just in-house LOB apps, but actual development tools like build systems - in the 2020s that don't support spaces in paths is mind-boggling.

Git security fixes released

nickleverton — Wed, 13 Apr 2022 14:13:50 +0000

Thanks for the headsup, it was very timely. All over the world, embedded system packagers are tearing their hair out wondering why their builds are now failing, because git under fakeroot now fails with a complaint about unsafe directories when image assembly scripts try to capture the hash for HEAD of the build tree.

Git security fixes released

jdisnard — Wed, 13 Apr 2022 13:27:19 +0000

> On Unix it's possible to set PS1 to use git, but it's not something I've actually seen used, so most users would need to proactively invoke git to get into trouble.

According to my anecdotal experience, it's very very common. I've seen dozens of developers have such a prompt in their bash or zsh shell environment.

The idea is so common that the upstream GIT project documented how it works:
https://git-scm.com/book/en/v2/Appendix-A%3A-Git-in-Other-Environments-Git-in-Bash

Git security fixes released

mathstuf — Wed, 13 Apr 2022 12:59:25 +0000

And it's often necessary to get paths short enough for some build systems or projects. Or those that don't support spaces in paths (yay "Documents and Settings").

Git security fixes released

mathstuf — Wed, 13 Apr 2022 12:57:25 +0000

Doing VCS detection is certainly common enough. I've been using zsh's vcs_info for 12 years or so. There are all kinds of blog posts on how to do it too (though they've become less necessary now that git has its own PS1 support).

Git security fixes released

k3ninho — Wed, 13 Apr 2022 12:55:49 +0000

>On Unix it's possible to set PS1 to use git, but it's not something I've actually seen used, so most users would need to proactively invoke git to get into trouble.

I have a lot of colleagues who do this, and the Ubuntu ec2 I'm running hash a .bashrc that pulls in /usr/lib/git-core/git-sh-prompt to add the branchname to the shell prompt. I guess it's popular enough that core git package supplies the __git_ps1 function. "it's 1337, u kno."

K3n.

Git security fixes released

draco — Wed, 13 Apr 2022 12:48:17 +0000

In my experience, non-admin users can usually create directories directly in the root of the C: drive -- even in work environments.

Git security fixes released

tialaramex — Wed, 13 Apr 2022 12:11:32 +0000

Most Windows users I've seen who use command line git, do so from an environment in which PS1 is set. So, they don't need to actually run "git" to be impacted, every time they look around a directory with this problem they're screwed by their shell running git for them. "Git Bash" in particular is a ready to use environment with Git and Bash on Windows. Thus just going into C:\temp to look at a file is enough to trigger the vulnerability for those users.

On Unix it's possible to set PS1 to use git, but it's not something I've actually seen used, so most users would need to proactively invoke git to get into trouble.

The other vulnerability is specific to the Uninstaller utility for Git on Windows and so doesn't exist on platforms where that's not how you uninstall software.

Git security fixes released

nye — Wed, 13 Apr 2022 10:19:35 +0000

It stops traversing upwards once it reaches a directory owned by a different user to the one where it started.

The thing that confuses me is why all the talk about this seems to imply that it's especially bad on Windows, which seems rather counterfactual - on Windows you're a great deal less likely to be using a directory under some shared common directory like /tmp, so this is essentially limited to "attacks" from an administrator who can change system directories. But an administrator could just modify your files without bothering with this "exploit", so...

Git security fixes released

ceplm — Wed, 13 Apr 2022 09:32:53 +0000

I am sorry, but this is a horrible changelog message. I am not that much interested in what’s the original problem, but I would like to learn what is the solution provided by the new change.

So, what has actually changed?

Git security fixes released

tamiko — Wed, 13 Apr 2022 08:39:25 +0000

It is a bit surprising that this well known behavior has been identified as a security vulnerability now and not years ago. But better late than never.

For example, one a Linux machine user A can simply create a git repository in /tmp and when user B comes along who happens to have a shell command line showing a git repository status it is game over. I remember discussing this with someone years ago...