Hacker News Reader

Comment by eminence32 | Hacker News Reader

> Merely navigating to such a space with a Git-enabled `PS1` when there is a maliciously-crafted `/scratch/.git/` can lead to a compromised account.

I'm curious about this -- what's the attack vector here?

View on HN

The key is the "Git-enabled `PS1`". PS1 is an environment variable recognised by common shell programs (such as bash) that configures the shell prompt. Git often installs its own glue into the prompt that ends up running a Git executable to discover such things as the current branch name and how many changed files. The vulnerability is that it's possible to add malicious things to .git/config that the git executable will pick up and call/run, even on simple operations like displaying the prompt.

in other news, access to a users account gives them access to a users account

In earlier news, it's unexpected that "cd directory" will give the directory owner access to your account.

Tbh I thought this was pretty obvious.

Git hooks have always been sketchy as hell.

Can't stand the Mac specific shit my co-workers keep dumping in there.

Wait, I thought git hooks aren't pulled from remote.

Wouldn't untrusted git hooks mean that git verify-* are useless since you're already running untrusted code?

On my current app we use husky which installs hooks when you do a yarn install

You are in luck, it seems: https://blog.typicode.com/husky-git-hooks-autoinstall/

(Though not completely sure.)

Just to react to:

>Wait, I thought git hooks aren't pulled from remote.

You are correct. They are not. Other tools may auto-install them (I hate it), but git does not ever.

This is really dismissive. Unexpected execution is not a users fault, and can happen for a variety of reasons (you telling me you’ve never unzipped a full git path someone sent you?)

No? Why would someone zip me a git repo? You can clone/push/pull directly between machines.

The source machine may not be set up as a server, or you may not have an account there. Sending a .zip file could be simpler than pushing the repo to a third system the receiver does have access to.

Sure, mailing binaries can also be the right thing sometimes, but just as with git repos, you definitely shouldn't be taking them from untrusted sources.

(and I bet that many people don't even realize how easy using `git push` to mirror a repo actually is, since git is very commonly used in a centralized fashion)

You’ve never unzipped a repo? That’s something I regularly do - clients can’t get me access to their scm, and it’s not worth the effort.

I don't think I ever did. I usually have an opposite problem and sometimes accidentally zip/tar a repo when it shouldn't be included (at least there's `--exclude-vcs`).

In any case, this doesn't sound like a good idea at all, as git hooks exist.

You’re right but also I’m mad that you’re right. I need to check what hook functions there are, but I knew about hooks and still overlooked them, which to me thinks that they’re non obvious. Maybe git should prompt you on first run, which is either whitelisted on your host or signed by your git keys or similar

Because linux has failed almost entirely to meet user level threats. Unlike android - which has per app permissions file permissions, linux is not there yet.

I know a lot of people are interested in better incapsulation for specific programs, and I know there's a lot of work being done in the area, but it's nowhere near as effective, in my opinion, as android and other systems.

linux follows the unix philosophy on this sort of. OK, you're a user, with some shell script, maybe git, maybe bash and it's PS1, I don't care, all I see as a kernel is that, you have permissions to edit this, upload this, send a packet, whatever, have fun!

From that perspective, nothing is wrong. That was my point. You could download s script that does 'rm / -Rf' and there's no security issue. User are given access to do as they please with files.

The issue is users can no longer reasonably trust the software on their system, from home-dialing marketing information and tracking, to having all sorts security issues in their virtual machines and sandboxes, running random code from websites constantly, we need a better way to encapsulate per file, per folder, per camera, per whatever, permissions, implemented at a system level.

You can download a directory containing a script, cd into that repo and less the file. ‘cd foo’ should never trigger arbitrary command execution. Ever.

Containerisation wouldn’t solve this, bash or similar would almost always be fun with near limitless boundaries.

I beg to disagree. I like my https://direnv.net/

As long as it's strictly opt-in, it's fine. But it needs to be opt-in to be secure.

Hi!

I can cheerfully confirm that I've absolutely never:

- received a git repo as an archive from someone, and then

- changed to root with "su" before unpacking it somewhere, such that

- the chosen location was above the home directory layer of a multi-user system.

- in such a way that one of the directories of the /path/to/home path has a .git/ subdirectory as a direct child, and not as an unpacked-tarball/.git grandchild which would not be accidentally found by git. I.e. that one of these directories exists, which might be found by someone running "git" in their home:

  /.git
  /path/.git
  /path/to/.git
  /path/to/home/.git

rather than the more likely:

  /foo-project-123/.git
  /path/foo-project-123/.git
  /path/to/foo-project-123/.git
  /path/to/home/foo-project-123/.git

which will not be found by someone running "git" in their home directory.

If I did such a thing, I'd care more about what happens when I happen to step on one of the malicious hooks in that repo as root, and less about what happens if users step on it.

Good for you. You are not everyone.

There are far more likely accidents that the superuser can perpetrate, that we do not compensate for with silly logic in applications.

Superuser could download some malware and put it into the system PATH. OK, so let's not execute anything in the PATH, unless it is owned by us.

/bin/ls? Not owned by me, don't trust it.

Okay.

What do you mean by “git often installs”. Git does not install anything. Developers configure their PS1 or install something that does.

on windows the default git package (ie. git bash) installs a bash terminal that has the git PS1 enabled.

The default shell on Mac and a lot of Linux distros do this installation, as well as git for windows.

Can you explain? A shell isn't supposed to be installing things on it's own... (Assuming you're talking about zsh or bash when you say shell.)

Yeah, even zsh doesn't do this by default

There's a lot of different ways to install Git on a lot of different OSes, and some of them put helper shell scripts down by default.

That isn't Git installing things though, that is third-party Git distributions bundling things along with Git.

I'm aware, which is why I phrased it like I did. A lot of folks don't know or understand the distinction when they're installing Git (or installing something else that happens to bring Git with it).

The overarching point is that the shell itself is not installing anything at shell runtime. E.g. it's the Git Bash installer, not opening `bash`.

It's not doing it on it's own, I believe what they're referring to is a a pre-configured distribution of a shell packaged with some distributions of Git, for some OSes.

Can you say what Linux distro does this? Seems like very poor taste for a command shell to be assuming one particular SCM.

Command shells assume a lot of things, such as your preferred text editor, locale, input and output devices, etc. Most of these things can be configured, but the default install makes assumptions based on common configurations.

On Ubuntu, and I presume therefore many debian derivatives and debian itself, `apt get git` will install a file called `/usr/lib/git-core/git-sh-prompt` (dpkg -S /usr/lib/git-core/git-sh-prompt).

This script allows you to see repository status in your prompt. It comes with 5 utility functions that AFAIKS are usable in all common shells:

    __git_ps1_show_upstream ()
    __git_ps1_colorize_gitstring ()
    __git_eread ()
    __git_sequencer_status ()
    __git_ps1 ()

On Ubuntu those aren't installed into bash by default, but you need to add it yourself to your ~/.bashrc (or zsh or whatever, but those aren't the default in Ubuntu). The mechanisms aren't in place for a package to inject itself into your prompt, but it isn't unthinkable (though maybe unwanted) to have that. Similar to how tab-complete for bash is handled:

The `git` package installs a file `/usr/share/bash-completion/completions/git` which is picked up by bash, on Ubuntu, as a pluggable way to extend autocomplete on bash. Many packages do this, on my machine there's 960 files in there (ls -1 /usr/share/bash-completion/completions/ | wc -l).¹ It would be trivial to have a similar mechanism for `bash_rc.d` which allows for a pluggable system to extend and modify your bash. Again, I presume this to be unwanted. I would oppose it.

¹ This is why I was surprised to learn that people like zsh for the reason that it offers tab-completion: I always presumed bash did that out of the box for any installed package already. Turns out it is Ubuntu (or better: Debian) doing this for me.

This feels like tab completion is a more serious attack vector then?

You need to be root to add a file to the basl completions directory, so not really.

There is no the bash completion directory; there is a default one which happens to be system wide and only root writable (as should be the case in a combination of such circumstances).

Fish allows for custom ones in ~/.config/fish and there is zero reason you cannot install custom ones in ~/.bashrc or user writable (on macOS) /usr/local

Anything installed through `apt` could be an attack vector, but that is silly, really.

If we start treating "apt get will install software that can change my system" as a security issue, we should stop using all electronic devises right now.

And yea: I know, we should have sandboxing, isolation, chroot and whatnot. And we are heading there. Yet in 2022, the vast majority of computers, servers and such are installed using package managers which install packages that have access to all the system. If you count mobile devises amongst "computers" then I guess a majority (Android) does have sandboxing in packages, which solves this particular issue.

> It would be trivial to have a similar mechanism for `bash_rc.d` which allows for a pluggable system to extend and modify your bash.

There is /etc/profile.d/ which serves much the same purpose for login shells, though PS1 is normally a non-exported shell variable so it wouldn't be inherited. You can configure most other aspects of the shell this way, however. And there is also /etc/bash.bashrc which is read before ~/.bashrc, though it doesn't provide a convenient directory for drop-in scripts by default the way /etc/profile does. It would be trivial to add that if desired. (All based on Debian; YMMV.)

The shell doesn't assume a particular one, but if it's in a directory where a supported one is present, then it enables those extras.

> Git does not install anything.

It's literally a Git installer.

Yeah, I'm familiar with PS1, but I was a bit surprised to learn that simple things that a PS1 script might do (git status, perhaps) are attack vectors. It seems that one big concern is the core.fsmonitor option (which I just learned about now). From the git-config man page:

> 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.

> …PS1 script…

That’s where you should have been concerned. Just typing a bare return will run some arbitrary code, as you, wherever you might be in the filesystem. If all of that isn’t under your control, someone can do anything to you.

Damn. So the script I use to put the branch name in my prompt is bad? Could I solve this by making .git/config readonly for non sudo?

The concern here is more on `sudo vi /.git/config` than `sudo vi ~/.git/config`. Someone adding a "root" git config that you don't expect/intend to exist.

If you think things are locked down strongly enough with sudo and never install anything that might add root files you don't expect you are most likely safe.

This release also adds an environment variable you can set that makes certain that git has a "ceiling" that it never crosses when checking for .git/config files. The idea being that you'd never want git to look above `/user/*/` for instance, as you'd never expect to have a "machine-wide" git config.

This article on a CVE for git published today has details on the vulnerability: https://github.blog/2022-04-12-git-security-vulnerability-an...

Ok, we've changed to that from https://github.com/git/git/commit/8959555cee7ec045958f9b6dd6.... Thanks!

I feel like title doesn't really focus on the specific behaviour change (not operating in a non-owned directories) that will be affecting a lot of CI/CD, which is what I was interesting in seeing discussion on.

You basically substituted both the link and the title to a completely different one after long time people discussing that specific link and title. Now half of the comments don't really make sense at the first glance.

The original link barely made any sense and many of those comments were comments without the useful context. The root cause here is the iffy submission, not the outdated comments or the change to a more meaningful link.

The original title was "Git 2.30.3 will not operate in non-owned directories" which makes perfect sense to me, and the link provides a good explanation of the security problem. What barely makes any sense about it?

It's a made-up title linking to some random commit. The new link tells you it's a fix for a vulnerability, the details, its CVE, affected platforms and use cases, etc, etc. The other thing doesn't.

The title was "made up", I'll give you that, but it's a pretty good paraphrase of the commit title to add context.

The old link also tells you it's a fix for a vulnerability, and also explains how it affects all platforms, and also talks about the use cases etc etc.

The only thing it doesn't have is a CVE number, which I don't think is all that important.

The official announcement tells your that there's a vuln, it's considered important enough to break things and that it's out right now. The other thing tells you someone committed something a few weeks ago. The missing context also helps drive a lot of under-informed grumpy threads, rather than bettter-informed grumpy comments/threads. There'd have probably been fewer grumpy threads with the better link.

They both say right at the top that it's a vulnerability, and the old title put the breakage front and center. So I don't know what you mean by missing context.

The context of one is 'someone committed a thing a few weeks ago and it does a thing, according to someone posting to HN'. The context of the other is 'one of the biggest git users on the planet tells you there's git vuln, fix out right now'.

Git itself removing an ability should tell you that it's a big deal even more than "one of the biggest git users on the planet".

And again, first line says it's a vulnerability. "it does a thing, according to someone posting to HN" is a big fat strawman.

It's really more of a Stay-Puft Marshmallow Man.

I don't know, it really doesn't sound like a real CVE - maybe add some setting I guess for those worried? Others bring up good points, if your attacker can write to C:\ you probably have other issues.

On top of that, it breaks completely valid functionality - someones 'bug' is someone elses feature.

Yeah, that happens sometimes, but if the second link is more suitable then the discussion eventually adapts for the better. So the only real question is which link is more suitable.

One principle I like to use is to assume readers are smart—e.g. in this case, that readers are smart enough to figure out that there are two relevant links to the comments. Of course randomness is also a factor, but it mostly all works out.

Ah ok this was the real link. The top level link to github.blog doesn't seem to have anything that this link here has. Please change it back.

Sorry for the confusion! I've added https://news.ycombinator.com/item?id=31016938 at the top of the thread so people will see both links.

Repos can have precommit hooks, which are just executables (usually executable she'll scripts, but anything will do) that will run (as your user) on commit, checkout, etc.

I feel like this change is a far bigger one than thought, and it's gonna break some workflows, such as mine where I have a git repo that's shared between multiple "users" that I run applications as. I'm glad I've not gotten too far into this project. The next step is just to keep doing a pull/push cycle on every commit, but it's a bit more of a pain to make that happen.

https://git-scm.com/docs/git-config/2.35.2#Documentation/git...

> I feel like this change is a far bigger one than thought, and it's gonna break some workflows

Breaks the CI system for perl, for example.

.git/config sets fsmonitor to malware.exe and boom.

Can you do this to a GitHub hosted repo?

No, the .git directory is not cloned. But if the repo is already on disk it can be game over.

Though you could have a repository on Github that contains a subdirectory that is a malicious bare Git repo. So doing:

``` git clone github.com/foo/bar cd bar/subdir/ ```

is unsafe with a Git PS1. See https://offensi.com/2019/12/16/4-google-cloud-shell-bugs-exp...

Looks like git complains of invalid paths when you try that.

Just because the cli won’t add doesn’t mean it may not be possible.

I was able to manually construct a commit with a .git subdirectory using `git mktree` and `git commit-tree`, but Git still refused to create the .git subdirectory in the index or working copy:

  [testrepo]$ git checkout --orphan test-branch
  
  [testrepo]$ git update-ref HEAD f4da9cde406a7b80d99694b5f8d369a8dd6e5a7d
  
  [testrepo]$ git ls-tree -r HEAD
  100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391    a/.git/config

  [testrepo]$ git show
  commit f4da9cde406a7b80d99694b5f8d369a8dd6e5a7d (HEAD -> test-branch)
  Author: <<redacted>>
  Date:   <<redacted>>

      WIP2

  diff --git a/a/.git/config b/a/.git/config
  new file mode 100644
  index 0000000..e69de29

  [testrepo]$ git status
  On branch test-branch
  Changes to be committed:
    (use "git restore --staged <file>..." to unstage)
          deleted:    a/.git/config

  [testrepo]$ git restore --staged a/.git/config
  error: invalid path 'a/.git/config'
  error: pathspec 'a/.git/config' did not match any file(s) known to git

  [testrepo]$ git reset --hard HEAD
  error: invalid path 'a/.git/config'
  fatal: Could not reset index file to revision 'HEAD'.

So it looks like even if you do try to check out a tree with an unexpected .git subdirectory it won't actually be created in the filesystem.

What do you mean?

Or create a mercurial repo that contains a .git directory, and rely on finger memory making them run git immediately after cloning...

I’ve done that a few times. I’ve typed “git foo” while mentally thinking about “svn foo”, and after a few hours working on a project that still uses svn I will start making the opposite substitution too.

We use the grunt build tool. More than a handful of times, I've tried to get grunt to do a merge, or git to build the app.

Finger memory, or just a shell configured to run `git status` before every command, as some people have.

And besides a Mercurial repo, it could also be a tarball or zip file…

Quite a dangerous situation.

I'm guessing the goal is to lock down git's security for any potential future vulnerability. If some security issue was discovered with git that could be exploited with a malicious .git directory structure, requiring git directories to be owned by the logged-in user will reduce the impact.

Or, an exploit was discovered, and quietly patched out recently

Oh, there it is: https://github.blog/2022-04-12-git-security-vulnerability-an...

Replying to myself again, apparently the thread title changed from "hey git won't let you do this thing anymore" to "here's the CVE", and changed the link too. sorry for the confusion

Couldn't that be mitigated by git adding a --readonly option that prevents any write operations? Then just use that option for any PS1 executables?

core.fsconfig is supposed to be read only, but since it’s an external process, git doesn’t know what really happens in there, so it can’t propagate any —-readonly enforcement.

That's where some sandboxing could help, maybe.

Reply on news.ycombinator.com