21 ms·

Git security vulnerability announced

549 points by delsarto 4y ago

jra_samba 4y ago
is_path_owned_by_current_uid(const char *path) isn't symlink safe given a multi-component path. Symlinks, the poisonous gift that keeps on giving.
- zionic 4y ago
  I’d love an OS that didn’t even support those.
  - userbinator 4y ago
    DOS? Non-NT Windows?
  - haolez 4y ago
    I think that was one of Plan9's selling points.
- sippycup6 4y ago
  I do not understand why symbolic links are "poisonous"? Can I get some context?
  - Thorrez 4y ago
    I think it's just that they're tricky when it comes to ownership. People who write code that depends on some type of file or directory ownership for security often don't think about the ways symlinks can be used to bypass their security model. You can sort of think of a symlink as having 2 owners: the user that owns the symlink itself, and the user who owns the file pointed to by the symlink. One of those owners might be an attacker, so every time you interact with a file, you have to think "this file might be half-owned by an attacker, and half-owned by a victim".
    - sippycup6 4y ago
      Thank you!
    - mishafb 4y ago
      Daemons that care about security setuid temporarily before opening a file and then setuid back
      - jra_samba 4y ago
        That doesn't always fix it. An attacker can race you to make you write something in a place you didn't intend or expect unless the application is incredibly carefully written. And by "incredibly" I mean beyond the scope of human endeavour :-).
  - jra_samba 4y ago
    I'm going to be giving a talk at SambaXP this year (it's virtual, so you only need to register to attend) explaining why IMHO symlinks have utterly broken the POSIX filesystem API, making it impossible for application developers to write secure applications. https://sambaxp.org/ https://sambaxp.org/ It's not just a whine, I'm also going to make some suggestions for fixing it :-).
- Thorrez 4y ago
  Are you saying this was the bug that was fixed, or that is is a new bug, that's not fixed yet?
  - jra_samba 4y ago
    I'm not sure it's a new bug, just that if you look up that call you'll find if it's used with any path containing a "/", then it can be raced to check somewhere other that the place the original author intended. That's why symlinks MUST die.
- deleted 4y ago
  [deleted]
rst 4y ago
Well, depending on exactly how much this blocks, this could get pretty awkward -- typing 'git log' in a repo owned by someone else can be awfully handy, even if file system permissions block changing it at all, and putting together a list of all places you might want to do this in advance could get pretty awkward. (Not running hooks, or allowing operations that would trigger them, from non-owned directories would preserve some of this usage, and still at least mitigate the dangerous cases somewhat.) It's also not entirely clear to me what this does to site-wide shared remotes, though I suppose if they can be listed in system config, it's at least not a per-user hassle.
- thelopa 4y ago
  I do this quite often, actually. I have my NixOS system config officially stored in /etc/nixos/ and owned by root. I have a clone that lives in my home directory for WIP changes, but builds always run out of the official copy. Sometimes it’s convenient to quickly run some read-only commands directly in the official copy
  - db48x 4y ago
    Ultimately you own that repository so just set it as safe in your config. Similarly if you are looking at a coworker’s repository then you can probably trust them. It’s only when you start sharing a computer with people that you don’t really know that you have a problem.
- zeroimpl 4y ago
  Yes I’ve run “git status”, “git log” and “git diff” on other people’s repo’s plenty of times to help debug things, so it’d be sad to see this stop working. It seems some basic readonly operations should still be supported.
  - krick 4y ago
    It doesn't even have to be a repo. I do git diff --no-index all the time on arbitrary files, because it's simply configured the way I want, unlike some default diff or whatever command, that I don't even remember how to properly use.
eminence32 4y ago
> 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?
- geoffeg 4y ago
  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.
  - hadlock 4y ago
    Or, an exploit was discovered, and quietly patched out recently
    - hadlock 4y ago
      Oh, there it is: https://github.blog/2022-04-12-git-security-vulnerability-announced/ https://github.blog/2022-04-12-git-security-vulnerability-an...
      - hadlock 4y ago
        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
- tedunangst 4y ago
  .git/config sets fsmonitor to malware.exe and boom.
  - hsbauauvhabzb 4y ago
    Can you do this to a GitHub hosted repo?
RayJSeth 4y ago
This certainly came as a surprise to my team today. We operate some number of repositories and the majority of them use https://github.com/actions-ecosystem/action-get-latest-tag https://github.com/actions-ecosystem/action-get-latest-tag - or more specifically, a fork of that repo which more or less works the same way. Midday today our CI/CD started failing. We must have hit this so soon because the `apk add git` in that Dockerfile grabbed the new git version. Evidently the SID that ultimately executed the git command inside the included actions' dockerfile was not the same as the one that owned `/github/workspace` on the runner. We were able to patch around using the new `safe.directory` option, but I'm curious to see if there's more fallout since CI/CD environments in particular create this sort of shared repository.
- __turbobrew__ 4y ago
  This is why I pin all dependencies in CI/CD.
- deleted 4y ago
  [deleted]
  - deleted 4y ago
    [deleted]
hsbauauvhabzb 4y ago
This is silly. Fix PS1, I can’t trust all repos I clone. I also want to cross-user access git log/blame etc.
- deleted 4y ago
  [deleted]
- deleted 4y ago
  [deleted]
- megous 4y ago
  Can't you? What kind of foreign code can be executed that way? Clonning will not copy .git/hooks/ nor .git/config which is the main danger here, I guess. But I'd sure want to hear about other risks. Maybe an env variable to disable hooks execution and .git/config parsing would be nice to have for safer use of git repositories you didn't clone yourself as part of shell prompt customizations.
  - hsbauauvhabzb 4y ago
    Git clone doesn’t mean I’m blindly executing the code inside it.
- megous 4y ago
  Also, looks like you didn't read the linked page. The first thing there is a git config option to disable this check on select directories.
  - saagarjha 4y ago
    Obviously adding every single repo you will ever work with into the config is not workable.
- kadoban 4y ago
  You don't have to trust repos you clone, if I understand correctly. You just need to trust ones you're given in other ways. The difference is, clone won't let you set up arbitrary config (or malformed internal data or etc.)
deleted 4y ago
[deleted]
bin_bash 4y ago
Doesn’t homebrew typically get setup as a different user? How’s that going to work?
- saagarjha 4y ago
  Homebrew typically changes the ownership of the directory to the current account.
  - rswail 4y ago
    Homebrew's mecahnisms are inherently "fail-unsafe". It really has no excuse.
  - bin_bash 4y ago
    On install or execution? What happens if there is more than 1 user on the box?
yuliyp 4y ago
This feels like a thing that should be introduced default-off, allowing users to opt in to it first, and once it's been in place update the default, rather than break things suddenly when updating without being able to share a git config between systems which don't upgrade simultaneously.
- deleted 4y ago
  [deleted]
Matheus28 4y ago
Shouldn't `safe_directory_cb` be checking the key parameter? It's ignoring it completely. So any unrelated config that has a directory in its value will also mark it as safe. Unless I'm misunderstanding something?
- db48x 4y ago
  That does seem like a mistake, upon a cursory examination.
  - Matheus28 4y ago
    I submitted a PR on github https://github.com/git/git/pull/1235 https://github.com/git/git/pull/1235. Supposedly there's a bot who will send an email, but I don't have permissions to use it... mhm...
    - db48x 4y ago
      Fun :) Going to send an email the old–fashioned way?
      - Matheus28 4y ago
        Gonna beg in the irc channel for git so they give me access to that bot. God forbid I have to format a patch the way they want me to
        db48x 4y ago
        lol :)
        vtbassmatt 4y ago
        You probably know this, but for anyone else following this thread: the bot is https://gitgitgadget.github.io/ https://gitgitgadget.github.io/.
- waynecochran 4y ago
deleted 4y ago
[deleted]
ab-dm 4y ago
Ah, so this is why my simple github release action randomly stopped working today... awesome
- deleted 4y ago
  [deleted]
- db48x 4y ago
  Surprise!
dataflow 4y ago
I feel like this doesn't have much to do with Git specifically. Seems to me like PS1 needs to avoid accessing files that aren't owned by the current user. Easier said than done though...
- deleted 4y ago
  [deleted]
- db48x 4y ago
  That’s partly true, but it is more relevant to Git than to other things because there are malicious ways to configure a git repository that will end up running programs written by someone else under your user id.
  - legalcorrection 4y ago
    Why are you setting your ps1 to run arbitrary code in any directory? Don’t do that!
    - db48x 4y ago
      It’s one step more indirect than that. If I want my prompt to tell me what branch is checked out, I can have it include the output of running `git branch`, for example. Unbeknownst to me, running `git branch` can cause git to run programs specified in the git repository’s config file. It’s not normally a problem of course, because I am using my own computer with all of my own git repositories. But it can be a problem if the computer is shared with others.
      - zarzavat 4y ago
        > `git branch` can cause git to run programs specified in the git repository’s config file This is the real vulnerability. Why is git branch running random external programs?
        dataflow 4y ago
bloaf 4y ago
Deep inside some large enterprise company: Jr Engineer: "Hey, I know we've always managed our little dotnet application via email and shared-network-drive, but I've been reading about a thing called "git" that we should probably use." Sr Engineer: "Change is scary and bad, also we are not a software company. We're not going to learn some newfangled whatsit. Just email me the .vba files when you want me to review the changes with the one copy of visual studio 2008 that our team has access to." Jr Engineer: "C'mon, give it a chance! We can leave everything the way its always been, but have better tracking of changes. Remember that time Bruno hard coded the tool to point to the C: drive? Git would let us just undo that, instead of having to search our emails for the last-most-recent version." Sr Engineer: "Ok fine, I've got 10 minutes, show me." Jr Engineer: "Ahh! Well I just got it installed, so let me go to the network drive... and then I think I have to git init our project folder... huh? Let me just... Maybe if I..." Sr Engineer: "Times up! Looks like this "git" thing isn't compatible with our setup after all. Those modern dev types never make anything that works in a real enterprise environment."
- codetrotter 4y ago
  Time to preemptively post this to StackOverflow and self-solve the question for some of that sweet sweet StackOverflow rep. jk
- sosodev 4y ago
  The premise of this story is one I lived. I was a web dev intern for a local government office and they actually emailed each other zips of dotnet apps. The only difference is that my git pitch went really well and they promised they would start using it. They never started using it.
  - legalcorrection 4y ago
    Local government software dev is making half of what they could make doing barely anything at a private sector operation. Not surprising there’s an IQ problem.
    - 4y ago
saagarjha 4y ago
I shouldn’t ask too much of an open source project, etc. etc., but this sounds like something Git should fix themselves rather than just outright disabling. “I want to go into a directory and run git log” is kind of a simple thing to want to do and to not be able to do that sucks. It’s easy to pontificate on this forum but having a “safe” git that doesn’t automatically run hooks or whatever seems like the way forward here, and useful outside of even just a “I want my PS1 to work”.
- db48x 4y ago
  On the one hand I agree with you; on the other hand that would entail enumerating all possible unsafe configurations. In general when designing a security measure you never want to try to enumerate everything that could be unsafe, because there is always an attacker who is more clever than you are who will think of something you left off the list.
  - saagarjha 4y ago
    Yes, but I feel like the other thing you need to keep in mind here is that this is going to be a massive pain for a lot of people, and they might end up doing things that are substantially worse for security, like refusing to update their git.
    - db48x 4y ago
      It’s possible, but I doubt it. 99% of people use a personal computer with just a single user account on it (or they use a phone with no git client, so let’s just think about git users for now). With only one real user account on the machine they are not very likely to encounter this security measure.
      - slavik81 4y ago
        Just about anybody that uses docker and mounts a volume is going to have multiple users accessing their files, even if there's only one real user.
hda2 4y ago
Was this change discussed publicly prior to merge? I think this is a big mistake. Build environments use separate users for security purposes. It's insane to decrease security for everyone by requiring a single user to do everything because some of your users want to have fancy terminal prompts. At the very least, let users configure this at a per-user level.
- tylersmith 4y ago
  Fixing RCE vulnerabilities isn't something that should be debated about publicly.
  - hda2 4y ago
    Nitpick: This isn't an RCE. An attacker would need 1) write access to a /local/ directory that the target will navigate to in his shell, and 2) convince the target to execute arbitrary git hooks in every directory (or parent directory) he visits by adding git to his shell's PS prompt. Besides, now that this security issue is patched, git devs should seek a proper solution to that doesn't break git and decrease security for everyone else.
  - wonnage 4y ago
    This isn't an RCE, you need to have control over the parent directory first which usually implies some sort of admin privileges already
  - saagarjha 4y ago
    This is just one way to fix the vulnerability. There are others, with different tradeoffs.
- deleted 4y ago
  [deleted]
krick 4y ago
This is bullshit. I mean, ok, you are concerned about somebody using git-enabled PS1. Guess what, not everyone is using git-enabled PS1. Unbelievable, right? I would even mock the fact that you are trying to protect users from the behavior they pretty much explicitly allowed, but this is pointless. Truth is, developers are doing something that can fuck up their system daily. Let's forget about wget | bash and copying completely untrusted git repositories (and it's pretty much guaranteed that everybody using git-enabled PS1 won't shy away from that). Just using composer or npm is enough to compromise your system. "Fixing" this is like introducing DRM: you cannot do arbitrary unsafe stuff without doing arbitrary unsafe stuff. And there simply are people out there, who want to do arbitrary unsafe stuff. But ok, let's not take it as an excuse. How about fixing git, then? I mean, actually fixing: making it possible to disable hooks & core.fsmonitor & whatever else they fucked up? No, right, let's just disable git instead. And if I'm reading this correctly, I'm not even allowed to say "I don't care" — I must explicitly mark every shared directory as trusted (I mean, safe.directory = '/' won't work unless / is actually a git directory, right?). I guess I just shouldn't update git until this "fix" is fixed. Or until git is forked.
- gruez 4y ago
  >Guess what, not everyone is using git-enabled PS1 not everyone is running on a multi-user system either (realistically speaking, most personal computers are single user). That doesn't mean microsoft/apple/linux doesn't care about escalation of privilege exploits. >Truth is, developers are doing something that can fuck up their system daily. Let's forget about wget | bash and copying completely untrusted git repositories (and it's pretty much guaranteed that everybody using git-enabled PS1 won't shy away from that). So because devs are doing dumb shit on a daily basis, they shouldn't fix security vulnerabilities? What if I'm not doing dumb shit? should I get hacked because I entered a malicious directory on a multi-user system? >I mean, actually fixing: making it possible to disable hooks & core.fsmonitor & whatever else they fucked up? No, right, let's just disable git instead. but then what if you need hooks? then you'll have to somehow manually enable it on a repo-by-repo basis, which also doesn't seem very convenient. At least with the ownership check it's transparent to most users. For people that use shared directories and/or network drive mounts, they can always whitelist the path.
mlindner 4y ago
Did the link get changed? I can't find anything of what anyone is talking about in this github.blog post.
- deleted 4y ago
  [deleted]
- wolfgang42 4y ago
  Yeah, dang updated the link: https://news.ycombinator.com/item?id=31010550 https://news.ycombinator.com/item?id=31010550 (it was previously https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9 https://github.com/git/git/commit/8959555cee7ec045958f9b6dd6...)
  - ineptech 4y ago
    Thank you, I was confused. I'm very curious if the people complaining about this change as being too paternalistic still feel that way after reading the full disclosure link.
    - SpicyLemonZest 4y ago
      Even after reading the full disclosure link, I'm pretty surprised to learn that a security boundary was intended here. I thought it was common knowledge that git did an uncontrolled search up the filesystem for a .git file, and it would never have occurred to me to run git on a machine where people I don't trust have write access.
      - Dylan16807 4y ago
        I was vaguely aware that git would search for .git directories. I had no idea that "git status" would run commands from such a directory.
deleted 4y ago
[deleted]
AussieWog93 4y ago
Fuck. Now the security guys are going to break all my shit.
- deleted 4y ago
  [deleted]
chrismarlow9 4y ago
Just run your git checkouts in a container and then link the volume to other containers! /s
- deleted 4y ago
  [deleted]
zmmmmm 4y ago
And .... there go probably tens of thousands of person-hours of human effort due to fixing this across huge numbers of systems. It's fascinating to me that we have people out there just casually making these kind of decisions with enormous cost implications with barely any thought to the downstream implications. Then meanwhile, we need approval in our org to claim a $30 taxi voucher as an expense.
- rectang 4y ago
  It boggles the mind that someone thinks security decisions like these are made casually. Staying sane as an open source maintainer means ignoring such thanklessness as best you can.
  - zmmmmm 4y ago
    Perhaps "casual" is the wrong word since it does have a pejorative implication. Put more neutrally, what I find fascinating is the asymmetry b/w the weight of process applied compared to impact. I am sure the individuals concerned thought very hard about it (I could not find the discussion on the mailing list, but from what I can see it may have been kept off the public list due to the security aspect).
  - avgcorrection 4y ago
    Isn’t git Hamano’s day job?
- tgsovlerkhgsel 4y ago
  Are you talking about stuff getting broken by this fix, or the patching effort required? This is relatively low risk so I would expect the mitigation to consist of "let your existing automation update it".
  - zmmmmm 4y ago
    No, much more thinking of broken CI systems and other deployment scenarios where a shared user setup is presumed.
- deleted 4y ago
kgeist 4y ago
>This vulnerability affects users working on multi-user machines where a malicious actor could create a .git directory in a shared location above a victim’s current working directory If a malicious actor has access to the filesystem, isn't it a bigger problem? I remember Raymond Chen recounted in his blog that Microsoft usually dismisses vulnerability reports that start with "to use the exploit, you must have access to the machine". As he likes to say, "the gates are already open". If you already have access to the machine and can create files outside of your home directory, what stops you from causing even greater havoc?
- hsbauauvhabzb 4y ago
  Network shares on corporate networks come to mind, they don’t need to be git repositories either (and presumably chowning all files to 1000:1000 would hit the large majority of Linux users even with this fix)
- keyanp 4y ago
  In the case of a multi-user machine, e.g. in a library, you expect there to be low privilege users with filesystem access. This bug introduces a way for them to do privilege escalation and potentially run code as root, which you did not intend. Generally, you still want these additional protections even if you don't expect others to have access to a machine. Can't say if one or the other is a bigger problem. I think they are all components of having a secure system.
- deleted 4y ago
  [deleted]
- Dylan16807 4y ago
  > If you already have access to the machine and can create files outside of your home directory, what stops you from causing even greater havoc? These systems don't let you put files in other people's directories. You can only create things in a specific spot, and if that thing is a directory then you and only you can put files inside it. Sometimes the only thing you can make in that spot is a directory. (Other users can access those files if you explicitly add them to the permissions, of course.)
- 4y ago
alkonaut 4y ago
Don’t make tools use processes for “plug-in behavior”. Do one thing and do it well doesn’t really appeal to me to begin with but “let the first thing do the next thing on its own” is definitely a bastardization of that idea as well. Git has that Unix disease where the go to method of getting anything user configurable done with one program is launching another program. I’d much rather use tools that use huge convoluted script languages or good plug-in apis than tools that duct tape together with exit codes.
- Emanation 4y ago
  Modular design is bad for profit, amirite
- deleted 4y ago
  [deleted]
- rswail 4y ago
  > I’d much rather use tools that use huge convoluted script languages or good plug-in apis than tools that duct tape together with exit codes. The Unix "plug-in API" is pipes and exec and "everything is a file (descriptor)". A "good plug-in API" that doesn't support anything written outside the "huge convoluted script language" is not a plug-in API, it's an internal API of the "convoluted script language". "Do one thing and do it well doesn’t really appeal to me to begin with" means that you don't like the Unix model in general.
  - alkonaut 4y ago
    > means that you don't like the Unix model in general. Absolutely correct. While it does have benefits in some situations, writing cross platform command line tools isn’t a place where it shines.
  - avgcorrection 4y ago
    > "Do one thing and do it well doesn’t really appeal to me to begin with" means that you don't like the Unix model in general. Yes? Some people don’t like it.
nodesocket 4y ago
Will Ubuntu update to v2.35.2? My current install is using the elder v2.25.1: ubuntu@vpn1:$ git --version git version 2.25.1 ubuntu@vpn1:$ cat /etc/os-release NAME="Ubuntu" VERSION="20.04.4 LTS (Focal Fossa)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 20.04.4 LTS" VERSION_ID="20.04" HOME_URL="https://www.ubuntu.com/" SUPPORT_URL="https://help.ubuntu.com/" BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/" PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy" VERSION_CODENAME=focal UBUNTU_CODENAME=focal
- alophawen 4y ago
  Since the security bug is windows-specific, you could play it cool.
  - McP 4y ago
    CVE-2022-24765 is not Windows-specific.
  - faho 4y ago
    It's not windows-specific. It's just that the git-for-windows people are more receptive to the idea of this being a security bug. The behavior is cross-platform.
- hyperman1 4y ago
  Same for debian 11 bullseye, which is today on git 2.30.2 . I'm not too worried, if someone besides me manages to create a /home/.git, then I'm already owned. But it's a bit surprising.
  - m4lvin 4y ago
    I assume the patch will be backported and there will be a point release .30.? including it, but bullseye will probably sticks with 2.30. However, you can use bullseye-backports to get 2.34.1 if you want: See https://packages.debian.org/git https://packages.debian.org/git Edit: None of the debian versions have the patch yet: https://security-tracker.debian.org/tracker/CVE-2022-24765 https://security-tracker.debian.org/tracker/CVE-2022-24765
dotancohen 4y ago
Here is a quick fix to prevent system-wide exploits, salt to taste: $ grep GIT_CEILING_DIRECTORIES ~/.bashrc export GIT_CEILING_DIRECTORIES=$HOME:/var/www But malicious Git repos could still affect your user profile. You can harden that by putting all git repos in a sandbox, e.g.: export GIT_CEILING_DIRECTORIES=$HOME/sandbox
ntauthority 4y ago
The Windows-specific 'vulnerability' is weird. For one, it's part of the uninstaller, which isn't a common scenario, and secondly... C:\Windows\Temp isn't even writable by unprivileged users by default, it's not even readable by unprivileged users by default (on my relatively fresh Windows 11 system, at least).
- nicce 4y ago
  At least on Windows 10 and multi-user installations everyone can access C:\Windows\Temp How do you define unprivileged?
  - jve 4y ago
    On my Windows 10 machine, I can't access C:\Windows\Temp as unprivileged user. It makes me press Continue, which will invoke admin rights to set privileges for that folder.
    - saurik 4y ago
      That's because you don't have the permission to list the contents of the folder, but you should have permission to create files in it.
      - jve 4y ago
        True. Get-Acl C:\Windows\TEMP | select -ExpandProperty AccessToString CREATOR OWNER Allow 268435456 NT AUTHORITY\SYSTEM Allow ReadData, Synchronize NT AUTHORITY\SYSTEM Allow 268435456 NT AUTHORITY\SYSTEM Allow FullControl BUILTIN\Administrators Allow 268435456 BUILTIN\Administrators Allow FullControl BUILTIN\Users Allow CreateFiles, AppendData, ExecuteFile, Synchronize BUILTIN\IIS_IUSRS Allow ReadData, Synchronize
- logbiscuitswave 4y ago
  The thing about c:\windows\temp is you can’t modify another user’s files but you can create your own. It’s actually a _really_ common vector to exploit poorly written installers by dropping your own file (like a malicious dll or exe) into that directory as a low rights user in the hope that the high rights installer process will then load that code. That’s presumably what’s happening in this case.
rvwaveren 4y ago
Question for Mac users. Apple installs git with its command line tools and is currently at version 2.32. Is it wise to install git via Homebrew so that you can upgrade faster? Or are there some benefits from apple-git?
- antihero 4y ago
  I've always used homebrew for git.
- 1123581321 4y ago
  I’m not aware of any benefit to using the system git, as a user.
- rswail 4y ago
  I use the macports version. As far as I am aware, there are no advantages to using the Apple version.
  - antihero 4y ago
    Out of interest any reason to select macports over brew? Since I started using mac brew seemed like the done thing
- lprd 4y ago
  No benefits that I’m aware of. I prefer having the latest greatest, so I’ve always used the homebrew tap.
tommiegannert 4y ago
> Run the uninstaller under an administrator account rather than as the SYSTEM user How do I run something as SYSTEM? I thought I always ran as "me" or Administrator. Is this only likely to happen for deployment automation tools? > Avoid running the uninstaller until after upgrading Don't leave us with this cliff-hanger... Does the upgrade installer run the uninstaller first? (The original report doesn't have this bullet point.)
- beermonster 4y ago
  You can run it using something like psexec sysinternals tool or Process Hacker. But it’s not something someone would likely do by accident and it’s a bit orchestrated/non-obvious.
  - entrep 4y ago
    If the installer is deployed with SCCM or Intune it's very likely being executed in system context.
- AlbertoAsw10 4y ago
  af
  - dgellow 4y ago
    What does that mean?
    - tentacleuno 4y ago
      One is left to assume that he feels the cliffhanger alluded to by the parent comment is "as fuck".
- dgellow 4y ago
  Yeah, that's something that should be clarified. I'm almost certain `winget upgrade git` will run the uninstaller first...
wheybags 4y ago
This feels really pointless. If I can create /.git, I have root already. Any other parent-directory-escalation that I can think of would be so obscure as to be not worth caring about, and would also probably require having access to an already-higher-privilege account. And of course the unspoken: almost nobody uses git on multi user systems, and when they do, most of the time every single user already has sudo.
- deleted 4y ago
  [deleted]
Dave3of5 4y ago
Awesome thanks for that !
jwilk 4y ago
Related: https://blog.sonarsource.com/securing-developer-tools-git-integrations/ https://blog.sonarsource.com/securing-developer-tools-git-in... ("Securing Developer Tools: Git Integrations")
sharken 4y ago
Interestingly if you're on Windows, then Chocolatey is the better package manager to use. Microsoft's own package manager Winget only has v2.34.1 right now. Chocolatey https://community.chocolatey.org/packages/git#versionhistory https://community.chocolatey.org/packages/git#versionhistory Winget https://winget.run/pkg/Git/Git https://winget.run/pkg/Git/Git
- dgellow 4y ago
  winget.run isn't up to date, I do see 2.35.2 by running the winget CLI (note that `winget upgrade git` will run the uninstaller first). $ winget show git.git Found Git [Git.Git] Version: 2.35.2 Publisher: The Git Development Community Publisher Url: https://gitforwindows.org Publisher Support Url: https://github.com/git-for-windows/git/issues Author: Johannes Schindelin Moniker: git Description: Git for Windows focuses on offering a lightweight, native set of tools that bring the full feature set of the Git SCM to Windows while providing appropriate user interfaces for experienced Git users and novices alike. Homepage: https://gitforwindows.org License: GNU General Public License version 2 License Url: https://raw.githubusercontent.com/git-for-windows/git/main/COPYING Copyright: Copyright (C) 1989, 1991 Free Software Foundation, Inc. Copyright Url: https://raw.githubusercontent.com/git-for-windows/git/main/COPYING Installer: Type: inno Download Url: https://github.com/git-for-windows/git/releases/download/v2.35.2.windows.1/Git-2.35.2-64-bit.exe SHA256: 8d33512f097e79adf7910d917653e630b3a4446b25fe258f6c3a21bdbde410ca
  - sharken 4y ago
    Yes, you're quite right. For future reference the GitHub manifest page seems to be the better choice: https://github.com/microsoft/winget-pkgs/tree/master/manifests/g/Git/Git https://github.com/microsoft/winget-pkgs/tree/master/manifes...
    - dgellow 4y ago
      Yes, that's always good to check but given the size of the repository (the number of directory and files is just massive!) it can be really annoying to navigate. Often faster to just run the winget CLI somewhere. winget.run should add a link to the manifest directory, that would be useful.
pocholo 4y ago
dcow 4y ago
What is a scenario where you’d be running git in the subdirectory of one owned by a malicious user? Unless a machine is badly configured and administrated, when would one user ever have authority of ownership over /home or /opt or /? And if they have sudo privileges well then they have the authority to do whatever they want. Is this only an issue because of some Windows idiom? I’m somewhat dubious.
- dcow 4y ago
  I can think of shell prompt plus exploring /tmp, but the fix for this “vuln” doesn't address that issue and seems to be more of a problem with a prompt that automatically runs git in every directory.
fargle 4y ago
It mildly bugs me that things like this are reported as "Git Security Vuln". CVE-12345: insecure use of consumer grade operating system in multi-user role when expecting any form of real isolation CVE-12346: faulty system administration techniques, including running anything as SYSTEM, can cause things to run with elevated privileges CVE-12347: failure to secure root (C:) and important system directories can allow malicious actors to access them. This can be exploited to trick other parts of the system into doing ... things. I don't mind patching git for windows to workaround these things, but sheesh, the root cause of both of these is people using Windows incorrectly/insecurely.
- AndrewUnmuted 4y ago
  > people using Windows incorrectly/insecurely. Let me fix that for you: > people using Windows.
- deleted 4y ago
  [deleted]
usrbinbash 4y ago
When people ask me why I don't have a "git-aware" PS1, I shall point them to this CVE in the future.
- grumple 4y ago
  Would be pretty incredible if the git branch command had a vulnerability. On the other hand, having a git aware PS1 would also immediately alert you to the fact that a user had created a top level .git folder, thereby allowing you to prevent the first cve here.
  - usrbinbash 4y ago
    > having a git aware PS1 would also immediately alert you to the fact that a user had created a top level .git folder, And to the fact that someone other than me had write access to my disk, in which case git is probably the least of my worries.
dang 4y ago
All: the originally submitted URL was https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9 https://github.com/git/git/commit/8959555cee7ec045958f9b6dd6.... Readers are divided about which link is better, which probably means you should read both to understand the thread.
Shalomboy 4y ago
There's something super jarring about the format of this blog post. I think my brain has been trained to glaze over whenever it runs into corporate abstract art at the top of a page.
deleted 4y ago
[deleted]
legalcorrection 4y ago
The more I think about it, the more I think this is the right call. The only alternative would be something like falling back to running with no hooks and printing a warning to stderr indicating that there are disabled hooks. Actions that modify that repository should also be disabled in that case. Then there should be a command like 'git hooks trust' that adds the directory to the user's list of trusted folders.
kazinator 4y ago
If you can create a .git directory above a victim's home directory, then you're root. Or else, if you're not root, you're in messed up system. Whoever is root should go read some 40-year-old book on Unix about how it's supposed to be laid out. This is not a genuine security vulnerability; though of course, it's good to fix it. Here is how I would fix it. Forget about permissions and ownership entirely. There is a weaker, more powerful condition we can check. Ready? Git should terminate if it is executed from a subdirectory of a git repo that contains no tracked files according to the first .git/ directory that it finds while ascending the file system. If you're in a directory that contains no files that are tracked by the closest .git/ that can be found by walking up the stairs, then that directory has no relationship to that repo. Git should diagnose that and bail out. (It could allow files in that directory to be added to the index, but only with -f option to force it.) If git finds a .git/ dir, and that repo's index shows that at least one item in, or below, your working directory is in that repo's index, it should go ahead and work with it, regardless of ownership.
- xyse53 4y ago
  The issue isn't specific to home directories. /tmp, for example. Your suggestion may protect against accidents, but doesn't seem to me to do anything for deliberately malicious behavior.
  - kazinator 4y ago
    Right, so someone could create a malicious /tmp/.git. You then go to /tmp/experiment to do something and run some git commands. Easy fix: on boot, have the "rc" script create a root-owned /tmp/.git dummy file with r-------- permissions. Someone can also create a /tmp/foo/.git; but to be susceptible to that, you have to be under /tmp/foo. That's another user's directory. What are you doing in there? Serves you right. If /tmp/foo is your own, and someone planted a .git into it, that's your problem too: you're creating material in /tmp that is accessible to others, which is a security no-no. Probably, this should be fixed in the kernel: the kernel should not allow a regular user to create a hidden directory (i.e. name starting with ".") in /tmp. Or probably any hidden object. Such a fix is more general: it fixes the issue for any git-like program that walks up the tree looking for a special dot directory or file, including all such programs not yet written. The rule could be general, such that creating a hidden object in a directory is only allowed to the directory's owner, not just to anyone who has write permissions to the directory. In other words, if multiple users have write access to a directory, such as /tmp, but any other kind of directory, then they are not allowed to perpetrate hidden objects on each other (both because those things don't show up under "ls" without "-a" and because programs find those and react to them). In fact, I would go one step further and enforce the kernel rule that writing to an existing dot file is denied to anyone other than the owner that file, regardless of its write permissions.
silverwind 4y ago
This "fix" breaks deployments where files are checked out as the root user and then chowned to an app-specific user. Any subsequent action as the root user will fail. It seems they forgot to provide an exception for the root user or a way to disable this "feature" on a global level, instead of per-directory.
deleted 4y ago
[deleted]

549 points by delsarto 4y ago

is_path_owned_by_current_uid(const char *path) isn't symlink safe given a multi-component path. Symlinks, the poisonous gift that keeps on giving.

zionic 4y ago
I’d love an OS that didn’t even support those.
- userbinator 4y ago
  DOS? Non-NT Windows?
- haolez 4y ago
  I think that was one of Plan9's selling points.
sippycup6 4y ago
I do not understand why symbolic links are "poisonous"? Can I get some context?
- Thorrez 4y ago
  I think it's just that they're tricky when it comes to ownership. People who write code that depends on some type of file or directory ownership for security often don't think about the ways symlinks can be used to bypass their security model. You can sort of think of a symlink as having 2 owners: the user that owns the symlink itself, and the user who owns the file pointed to by the symlink. One of those owners might be an attacker, so every time you interact with a file, you have to think "this file might be half-owned by an attacker, and half-owned by a victim".
  - sippycup6 4y ago
    Thank you!
  - mishafb 4y ago
    Daemons that care about security setuid temporarily before opening a file and then setuid back
    - jra_samba 4y ago
      That doesn't always fix it. An attacker can race you to make you write something in a place you didn't intend or expect unless the application is incredibly carefully written. And by "incredibly" I mean beyond the scope of human endeavour :-).
- jra_samba 4y ago
  I'm going to be giving a talk at SambaXP this year (it's virtual, so you only need to register to attend) explaining why IMHO symlinks have utterly broken the POSIX filesystem API, making it impossible for application developers to write secure applications. https://sambaxp.org/ https://sambaxp.org/ It's not just a whine, I'm also going to make some suggestions for fixing it :-).
Thorrez 4y ago
Are you saying this was the bug that was fixed, or that is is a new bug, that's not fixed yet?
- jra_samba 4y ago
  I'm not sure it's a new bug, just that if you look up that call you'll find if it's used with any path containing a "/", then it can be raced to check somewhere other that the place the original author intended. That's why symlinks MUST die.
deleted 4y ago
[deleted]

rst 4y ago

Well, depending on exactly how much this blocks, this could get pretty awkward -- typing 'git log' in a repo owned by someone else can be awfully handy, even if file system permissions block changing it at all, and putting together a list of all places you might want to do this in advance could get pretty awkward. (Not running hooks, or allowing operations that would trigger them, from non-owned directories would preserve some of this usage, and still at least mitigate the dangerous cases somewhat.) It's also not entirely clear to me what this does to site-wide shared remotes, though I suppose if they can be listed in system config, it's at least not a per-user hassle.

thelopa 4y ago
I do this quite often, actually. I have my NixOS system config officially stored in /etc/nixos/ and owned by root. I have a clone that lives in my home directory for WIP changes, but builds always run out of the official copy. Sometimes it’s convenient to quickly run some read-only commands directly in the official copy
- db48x 4y ago
  Ultimately you own that repository so just set it as safe in your config. Similarly if you are looking at a coworker’s repository then you can probably trust them. It’s only when you start sharing a computer with people that you don’t really know that you have a problem.
zeroimpl 4y ago
Yes I’ve run “git status”, “git log” and “git diff” on other people’s repo’s plenty of times to help debug things, so it’d be sad to see this stop working. It seems some basic readonly operations should still be supported.
- krick 4y ago
  It doesn't even have to be a repo. I do git diff --no-index all the time on arbitrary files, because it's simply configured the way I want, unlike some default diff or whatever command, that I don't even remember how to properly use.

eminence32 4y ago

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

geoffeg 4y ago
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.
- hadlock 4y ago
  Or, an exploit was discovered, and quietly patched out recently
  - hadlock 4y ago
    Oh, there it is: https://github.blog/2022-04-12-git-security-vulnerability-announced/ https://github.blog/2022-04-12-git-security-vulnerability-an...
    - hadlock 4y ago
      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
tedunangst 4y ago
.git/config sets fsmonitor to malware.exe and boom.
- hsbauauvhabzb 4y ago
  Can you do this to a GitHub hosted repo?

RayJSeth 4y ago

This certainly came as a surprise to my team today. We operate some number of repositories and the majority of them use https://github.com/actions-ecosystem/action-get-latest-tag https://github.com/actions-ecosystem/action-get-latest-tag - or more specifically, a fork of that repo which more or less works the same way. Midday today our CI/CD started failing. We must have hit this so soon because the `apk add git` in that Dockerfile grabbed the new git version. Evidently the SID that ultimately executed the git command inside the included actions' dockerfile was not the same as the one that owned `/github/workspace` on the runner. We were able to patch around using the new `safe.directory` option, but I'm curious to see if there's more fallout since CI/CD environments in particular create this sort of shared repository.

__turbobrew__ 4y ago
This is why I pin all dependencies in CI/CD.
deleted 4y ago
[deleted]
- deleted 4y ago
  [deleted]

hsbauauvhabzb 4y ago

This is silly. Fix PS1, I can’t trust all repos I clone. I also want to cross-user access git log/blame etc.

deleted 4y ago
[deleted]
deleted 4y ago
[deleted]
megous 4y ago
Can't you? What kind of foreign code can be executed that way? Clonning will not copy .git/hooks/ nor .git/config which is the main danger here, I guess. But I'd sure want to hear about other risks. Maybe an env variable to disable hooks execution and .git/config parsing would be nice to have for safer use of git repositories you didn't clone yourself as part of shell prompt customizations.
- hsbauauvhabzb 4y ago
  Git clone doesn’t mean I’m blindly executing the code inside it.
megous 4y ago
Also, looks like you didn't read the linked page. The first thing there is a git config option to disable this check on select directories.
- saagarjha 4y ago
  Obviously adding every single repo you will ever work with into the config is not workable.
kadoban 4y ago
You don't have to trust repos you clone, if I understand correctly. You just need to trust ones you're given in other ways. The difference is, clone won't let you set up arbitrary config (or malformed internal data or etc.)

deleted 4y ago

[deleted]

bin_bash 4y ago

Doesn’t homebrew typically get setup as a different user? How’s that going to work?

saagarjha 4y ago
Homebrew typically changes the ownership of the directory to the current account.
- rswail 4y ago
  Homebrew's mecahnisms are inherently "fail-unsafe". It really has no excuse.
- bin_bash 4y ago
  On install or execution? What happens if there is more than 1 user on the box?

yuliyp 4y ago

This feels like a thing that should be introduced default-off, allowing users to opt in to it first, and once it's been in place update the default, rather than break things suddenly when updating without being able to share a git config between systems which don't upgrade simultaneously.

deleted 4y ago
[deleted]

Matheus28 4y ago

Shouldn't `safe_directory_cb` be checking the key parameter? It's ignoring it completely. So any unrelated config that has a directory in its value will also mark it as safe. Unless I'm misunderstanding something?

db48x 4y ago
That does seem like a mistake, upon a cursory examination.
- Matheus28 4y ago
  I submitted a PR on github https://github.com/git/git/pull/1235 https://github.com/git/git/pull/1235. Supposedly there's a bot who will send an email, but I don't have permissions to use it... mhm...
  - db48x 4y ago
    Fun :) Going to send an email the old–fashioned way?
    - Matheus28 4y ago
      Gonna beg in the irc channel for git so they give me access to that bot. God forbid I have to format a patch the way they want me to
      - db48x 4y ago
        lol :)
      - vtbassmatt 4y ago
        You probably know this, but for anyone else following this thread: the bot is https://gitgitgadget.github.io/ https://gitgitgadget.github.io/.
waynecochran 4y ago

deleted 4y ago

[deleted]

ab-dm 4y ago

Ah, so this is why my simple github release action randomly stopped working today... awesome

deleted 4y ago
[deleted]
db48x 4y ago
Surprise!

dataflow 4y ago

I feel like this doesn't have much to do with Git specifically. Seems to me like PS1 needs to avoid accessing files that aren't owned by the current user. Easier said than done though...

deleted 4y ago
[deleted]
db48x 4y ago
That’s partly true, but it is more relevant to Git than to other things because there are malicious ways to configure a git repository that will end up running programs written by someone else under your user id.
- legalcorrection 4y ago
  Why are you setting your ps1 to run arbitrary code in any directory? Don’t do that!
  - db48x 4y ago
    It’s one step more indirect than that. If I want my prompt to tell me what branch is checked out, I can have it include the output of running `git branch`, for example. Unbeknownst to me, running `git branch` can cause git to run programs specified in the git repository’s config file. It’s not normally a problem of course, because I am using my own computer with all of my own git repositories. But it can be a problem if the computer is shared with others.
    - zarzavat 4y ago
      > `git branch` can cause git to run programs specified in the git repository’s config file This is the real vulnerability. Why is git branch running random external programs?
      - dataflow 4y ago

bloaf 4y ago

Deep inside some large enterprise company: Jr Engineer: "Hey, I know we've always managed our little dotnet application via email and shared-network-drive, but I've been reading about a thing called "git" that we should probably use." Sr Engineer: "Change is scary and bad, also we are not a software company. We're not going to learn some newfangled whatsit. Just email me the .vba files when you want me to review the changes with the one copy of visual studio 2008 that our team has access to." Jr Engineer: "C'mon, give it a chance! We can leave everything the way its always been, but have better tracking of changes. Remember that time Bruno hard coded the tool to point to the C: drive? Git would let us just undo that, instead of having to search our emails for the last-most-recent version." Sr Engineer: "Ok fine, I've got 10 minutes, show me." Jr Engineer: "Ahh! Well I just got it installed, so let me go to the network drive... and then I think I have to git init our project folder... huh? Let me just... Maybe if I..." Sr Engineer: "Times up! Looks like this "git" thing isn't compatible with our setup after all. Those modern dev types never make anything that works in a real enterprise environment."

codetrotter 4y ago
Time to preemptively post this to StackOverflow and self-solve the question for some of that sweet sweet StackOverflow rep. jk
sosodev 4y ago
The premise of this story is one I lived. I was a web dev intern for a local government office and they actually emailed each other zips of dotnet apps. The only difference is that my git pitch went really well and they promised they would start using it. They never started using it.
- legalcorrection 4y ago
  Local government software dev is making half of what they could make doing barely anything at a private sector operation. Not surprising there’s an IQ problem.
  - 4y ago

saagarjha 4y ago

I shouldn’t ask too much of an open source project, etc. etc., but this sounds like something Git should fix themselves rather than just outright disabling. “I want to go into a directory and run git log” is kind of a simple thing to want to do and to not be able to do that sucks. It’s easy to pontificate on this forum but having a “safe” git that doesn’t automatically run hooks or whatever seems like the way forward here, and useful outside of even just a “I want my PS1 to work”.

db48x 4y ago
On the one hand I agree with you; on the other hand that would entail enumerating all possible unsafe configurations. In general when designing a security measure you never want to try to enumerate everything that could be unsafe, because there is always an attacker who is more clever than you are who will think of something you left off the list.
- saagarjha 4y ago
  Yes, but I feel like the other thing you need to keep in mind here is that this is going to be a massive pain for a lot of people, and they might end up doing things that are substantially worse for security, like refusing to update their git.
  - db48x 4y ago
    It’s possible, but I doubt it. 99% of people use a personal computer with just a single user account on it (or they use a phone with no git client, so let’s just think about git users for now). With only one real user account on the machine they are not very likely to encounter this security measure.
    - slavik81 4y ago
      Just about anybody that uses docker and mounts a volume is going to have multiple users accessing their files, even if there's only one real user.

hda2 4y ago

Was this change discussed publicly prior to merge? I think this is a big mistake. Build environments use separate users for security purposes. It's insane to decrease security for everyone by requiring a single user to do everything because some of your users want to have fancy terminal prompts. At the very least, let users configure this at a per-user level.

tylersmith 4y ago
Fixing RCE vulnerabilities isn't something that should be debated about publicly.
- hda2 4y ago
  Nitpick: This isn't an RCE. An attacker would need 1) write access to a /local/ directory that the target will navigate to in his shell, and 2) convince the target to execute arbitrary git hooks in every directory (or parent directory) he visits by adding git to his shell's PS prompt. Besides, now that this security issue is patched, git devs should seek a proper solution to that doesn't break git and decrease security for everyone else.
- wonnage 4y ago
  This isn't an RCE, you need to have control over the parent directory first which usually implies some sort of admin privileges already
- saagarjha 4y ago
  This is just one way to fix the vulnerability. There are others, with different tradeoffs.
deleted 4y ago
[deleted]

krick 4y ago

This is bullshit. I mean, ok, you are concerned about somebody using git-enabled PS1. Guess what, not everyone is using git-enabled PS1. Unbelievable, right? I would even mock the fact that you are trying to protect users from the behavior they pretty much explicitly allowed, but this is pointless. Truth is, developers are doing something that can fuck up their system daily. Let's forget about wget | bash and copying completely untrusted git repositories (and it's pretty much guaranteed that everybody using git-enabled PS1 won't shy away from that). Just using composer or npm is enough to compromise your system. "Fixing" this is like introducing DRM: you cannot do arbitrary unsafe stuff without doing arbitrary unsafe stuff. And there simply are people out there, who want to do arbitrary unsafe stuff. But ok, let's not take it as an excuse. How about fixing git, then? I mean, actually fixing: making it possible to disable hooks & core.fsmonitor & whatever else they fucked up? No, right, let's just disable git instead. And if I'm reading this correctly, I'm not even allowed to say "I don't care" — I must explicitly mark every shared directory as trusted (I mean, safe.directory = '/' won't work unless / is actually a git directory, right?). I guess I just shouldn't update git until this "fix" is fixed. Or until git is forked.

gruez 4y ago
>Guess what, not everyone is using git-enabled PS1 not everyone is running on a multi-user system either (realistically speaking, most personal computers are single user). That doesn't mean microsoft/apple/linux doesn't care about escalation of privilege exploits. >Truth is, developers are doing something that can fuck up their system daily. Let's forget about wget | bash and copying completely untrusted git repositories (and it's pretty much guaranteed that everybody using git-enabled PS1 won't shy away from that). So because devs are doing dumb shit on a daily basis, they shouldn't fix security vulnerabilities? What if I'm not doing dumb shit? should I get hacked because I entered a malicious directory on a multi-user system? >I mean, actually fixing: making it possible to disable hooks & core.fsmonitor & whatever else they fucked up? No, right, let's just disable git instead. but then what if you need hooks? then you'll have to somehow manually enable it on a repo-by-repo basis, which also doesn't seem very convenient. At least with the ownership check it's transparent to most users. For people that use shared directories and/or network drive mounts, they can always whitelist the path.

mlindner 4y ago

Did the link get changed? I can't find anything of what anyone is talking about in this github.blog post.

deleted 4y ago
[deleted]
wolfgang42 4y ago
Yeah, dang updated the link: https://news.ycombinator.com/item?id=31010550 https://news.ycombinator.com/item?id=31010550 (it was previously https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9 https://github.com/git/git/commit/8959555cee7ec045958f9b6dd6...)
- ineptech 4y ago
  Thank you, I was confused. I'm very curious if the people complaining about this change as being too paternalistic still feel that way after reading the full disclosure link.
  - SpicyLemonZest 4y ago
    Even after reading the full disclosure link, I'm pretty surprised to learn that a security boundary was intended here. I thought it was common knowledge that git did an uncontrolled search up the filesystem for a .git file, and it would never have occurred to me to run git on a machine where people I don't trust have write access.
    - Dylan16807 4y ago
      I was vaguely aware that git would search for .git directories. I had no idea that "git status" would run commands from such a directory.

deleted 4y ago

[deleted]

AussieWog93 4y ago

Fuck. Now the security guys are going to break all my shit.

deleted 4y ago
[deleted]

chrismarlow9 4y ago

Just run your git checkouts in a container and then link the volume to other containers! /s

deleted 4y ago
[deleted]

zmmmmm 4y ago

And .... there go probably tens of thousands of person-hours of human effort due to fixing this across huge numbers of systems. It's fascinating to me that we have people out there just casually making these kind of decisions with enormous cost implications with barely any thought to the downstream implications. Then meanwhile, we need approval in our org to claim a $30 taxi voucher as an expense.

rectang 4y ago
It boggles the mind that someone thinks security decisions like these are made casually. Staying sane as an open source maintainer means ignoring such thanklessness as best you can.
- zmmmmm 4y ago
  Perhaps "casual" is the wrong word since it does have a pejorative implication. Put more neutrally, what I find fascinating is the asymmetry b/w the weight of process applied compared to impact. I am sure the individuals concerned thought very hard about it (I could not find the discussion on the mailing list, but from what I can see it may have been kept off the public list due to the security aspect).
- avgcorrection 4y ago
  Isn’t git Hamano’s day job?
tgsovlerkhgsel 4y ago
Are you talking about stuff getting broken by this fix, or the patching effort required? This is relatively low risk so I would expect the mitigation to consist of "let your existing automation update it".
- zmmmmm 4y ago
  No, much more thinking of broken CI systems and other deployment scenarios where a shared user setup is presumed.
deleted 4y ago

kgeist 4y ago

>This vulnerability affects users working on multi-user machines where a malicious actor could create a .git directory in a shared location above a victim’s current working directory If a malicious actor has access to the filesystem, isn't it a bigger problem? I remember Raymond Chen recounted in his blog that Microsoft usually dismisses vulnerability reports that start with "to use the exploit, you must have access to the machine". As he likes to say, "the gates are already open". If you already have access to the machine and can create files outside of your home directory, what stops you from causing even greater havoc?

hsbauauvhabzb 4y ago
Network shares on corporate networks come to mind, they don’t need to be git repositories either (and presumably chowning all files to 1000:1000 would hit the large majority of Linux users even with this fix)
keyanp 4y ago
In the case of a multi-user machine, e.g. in a library, you expect there to be low privilege users with filesystem access. This bug introduces a way for them to do privilege escalation and potentially run code as root, which you did not intend. Generally, you still want these additional protections even if you don't expect others to have access to a machine. Can't say if one or the other is a bigger problem. I think they are all components of having a secure system.
deleted 4y ago
[deleted]
Dylan16807 4y ago
> If you already have access to the machine and can create files outside of your home directory, what stops you from causing even greater havoc? These systems don't let you put files in other people's directories. You can only create things in a specific spot, and if that thing is a directory then you and only you can put files inside it. Sometimes the only thing you can make in that spot is a directory. (Other users can access those files if you explicitly add them to the permissions, of course.)
4y ago

alkonaut 4y ago

Don’t make tools use processes for “plug-in behavior”. Do one thing and do it well doesn’t really appeal to me to begin with but “let the first thing do the next thing on its own” is definitely a bastardization of that idea as well. Git has that Unix disease where the go to method of getting anything user configurable done with one program is launching another program. I’d much rather use tools that use huge convoluted script languages or good plug-in apis than tools that duct tape together with exit codes.

Emanation 4y ago
Modular design is bad for profit, amirite
deleted 4y ago
[deleted]
rswail 4y ago
> I’d much rather use tools that use huge convoluted script languages or good plug-in apis than tools that duct tape together with exit codes. The Unix "plug-in API" is pipes and exec and "everything is a file (descriptor)". A "good plug-in API" that doesn't support anything written outside the "huge convoluted script language" is not a plug-in API, it's an internal API of the "convoluted script language". "Do one thing and do it well doesn’t really appeal to me to begin with" means that you don't like the Unix model in general.
- alkonaut 4y ago
  > means that you don't like the Unix model in general. Absolutely correct. While it does have benefits in some situations, writing cross platform command line tools isn’t a place where it shines.
- avgcorrection 4y ago
  > "Do one thing and do it well doesn’t really appeal to me to begin with" means that you don't like the Unix model in general. Yes? Some people don’t like it.

nodesocket 4y ago

Will Ubuntu update to v2.35.2? My current install is using the elder v2.25.1: ubuntu@vpn1:$ git --version git version 2.25.1 ubuntu@vpn1:$ cat /etc/os-release NAME="Ubuntu" VERSION="20.04.4 LTS (Focal Fossa)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 20.04.4 LTS" VERSION_ID="20.04" HOME_URL="https://www.ubuntu.com/" SUPPORT_URL="https://help.ubuntu.com/" BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/" PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy" VERSION_CODENAME=focal UBUNTU_CODENAME=focal

alophawen 4y ago
Since the security bug is windows-specific, you could play it cool.
- McP 4y ago
  CVE-2022-24765 is not Windows-specific.
- faho 4y ago
  It's not windows-specific. It's just that the git-for-windows people are more receptive to the idea of this being a security bug. The behavior is cross-platform.
hyperman1 4y ago
Same for debian 11 bullseye, which is today on git 2.30.2 . I'm not too worried, if someone besides me manages to create a /home/.git, then I'm already owned. But it's a bit surprising.
- m4lvin 4y ago
  I assume the patch will be backported and there will be a point release .30.? including it, but bullseye will probably sticks with 2.30. However, you can use bullseye-backports to get 2.34.1 if you want: See https://packages.debian.org/git https://packages.debian.org/git Edit: None of the debian versions have the patch yet: https://security-tracker.debian.org/tracker/CVE-2022-24765 https://security-tracker.debian.org/tracker/CVE-2022-24765

dotancohen 4y ago

Here is a quick fix to prevent system-wide exploits, salt to taste: $ grep GIT_CEILING_DIRECTORIES ~/.bashrc export GIT_CEILING_DIRECTORIES=$HOME:/var/www But malicious Git repos could still affect your user profile. You can harden that by putting all git repos in a sandbox, e.g.: export GIT_CEILING_DIRECTORIES=$HOME/sandbox

ntauthority 4y ago

The Windows-specific 'vulnerability' is weird. For one, it's part of the uninstaller, which isn't a common scenario, and secondly... C:\Windows\Temp isn't even writable by unprivileged users by default, it's not even readable by unprivileged users by default (on my relatively fresh Windows 11 system, at least).

nicce 4y ago
At least on Windows 10 and multi-user installations everyone can access C:\Windows\Temp How do you define unprivileged?
- jve 4y ago
  On my Windows 10 machine, I can't access C:\Windows\Temp as unprivileged user. It makes me press Continue, which will invoke admin rights to set privileges for that folder.
  - saurik 4y ago
    That's because you don't have the permission to list the contents of the folder, but you should have permission to create files in it.
    - jve 4y ago
      True. Get-Acl C:\Windows\TEMP | select -ExpandProperty AccessToString CREATOR OWNER Allow 268435456 NT AUTHORITY\SYSTEM Allow ReadData, Synchronize NT AUTHORITY\SYSTEM Allow 268435456 NT AUTHORITY\SYSTEM Allow FullControl BUILTIN\Administrators Allow 268435456 BUILTIN\Administrators Allow FullControl BUILTIN\Users Allow CreateFiles, AppendData, ExecuteFile, Synchronize BUILTIN\IIS_IUSRS Allow ReadData, Synchronize
logbiscuitswave 4y ago
The thing about c:\windows\temp is you can’t modify another user’s files but you can create your own. It’s actually a _really_ common vector to exploit poorly written installers by dropping your own file (like a malicious dll or exe) into that directory as a low rights user in the hope that the high rights installer process will then load that code. That’s presumably what’s happening in this case.

rvwaveren 4y ago

Question for Mac users. Apple installs git with its command line tools and is currently at version 2.32. Is it wise to install git via Homebrew so that you can upgrade faster? Or are there some benefits from apple-git?

antihero 4y ago
I've always used homebrew for git.
1123581321 4y ago
I’m not aware of any benefit to using the system git, as a user.
rswail 4y ago
I use the macports version. As far as I am aware, there are no advantages to using the Apple version.
- antihero 4y ago
  Out of interest any reason to select macports over brew? Since I started using mac brew seemed like the done thing
lprd 4y ago
No benefits that I’m aware of. I prefer having the latest greatest, so I’ve always used the homebrew tap.

tommiegannert 4y ago

> Run the uninstaller under an administrator account rather than as the SYSTEM user How do I run something as SYSTEM? I thought I always ran as "me" or Administrator. Is this only likely to happen for deployment automation tools? > Avoid running the uninstaller until after upgrading Don't leave us with this cliff-hanger... Does the upgrade installer run the uninstaller first? (The original report doesn't have this bullet point.)

beermonster 4y ago
You can run it using something like psexec sysinternals tool or Process Hacker. But it’s not something someone would likely do by accident and it’s a bit orchestrated/non-obvious.
- entrep 4y ago
  If the installer is deployed with SCCM or Intune it's very likely being executed in system context.
AlbertoAsw10 4y ago
af
- dgellow 4y ago
  What does that mean?
  - tentacleuno 4y ago
    One is left to assume that he feels the cliffhanger alluded to by the parent comment is "as fuck".
dgellow 4y ago
Yeah, that's something that should be clarified. I'm almost certain `winget upgrade git` will run the uninstaller first...

wheybags 4y ago

This feels really pointless. If I can create /.git, I have root already. Any other parent-directory-escalation that I can think of would be so obscure as to be not worth caring about, and would also probably require having access to an already-higher-privilege account. And of course the unspoken: almost nobody uses git on multi user systems, and when they do, most of the time every single user already has sudo.

deleted 4y ago
[deleted]

Dave3of5 4y ago

Awesome thanks for that !

jwilk 4y ago

Related: https://blog.sonarsource.com/securing-developer-tools-git-integrations/ https://blog.sonarsource.com/securing-developer-tools-git-in... ("Securing Developer Tools: Git Integrations")

sharken 4y ago

Interestingly if you're on Windows, then Chocolatey is the better package manager to use. Microsoft's own package manager Winget only has v2.34.1 right now. Chocolatey https://community.chocolatey.org/packages/git#versionhistory https://community.chocolatey.org/packages/git#versionhistory Winget https://winget.run/pkg/Git/Git https://winget.run/pkg/Git/Git

dgellow 4y ago
winget.run isn't up to date, I do see 2.35.2 by running the winget CLI (note that `winget upgrade git` will run the uninstaller first). $ winget show git.git Found Git [Git.Git] Version: 2.35.2 Publisher: The Git Development Community Publisher Url: https://gitforwindows.org Publisher Support Url: https://github.com/git-for-windows/git/issues Author: Johannes Schindelin Moniker: git Description: Git for Windows focuses on offering a lightweight, native set of tools that bring the full feature set of the Git SCM to Windows while providing appropriate user interfaces for experienced Git users and novices alike. Homepage: https://gitforwindows.org License: GNU General Public License version 2 License Url: https://raw.githubusercontent.com/git-for-windows/git/main/COPYING Copyright: Copyright (C) 1989, 1991 Free Software Foundation, Inc. Copyright Url: https://raw.githubusercontent.com/git-for-windows/git/main/COPYING Installer: Type: inno Download Url: https://github.com/git-for-windows/git/releases/download/v2.35.2.windows.1/Git-2.35.2-64-bit.exe SHA256: 8d33512f097e79adf7910d917653e630b3a4446b25fe258f6c3a21bdbde410ca
- sharken 4y ago
  Yes, you're quite right. For future reference the GitHub manifest page seems to be the better choice: https://github.com/microsoft/winget-pkgs/tree/master/manifests/g/Git/Git https://github.com/microsoft/winget-pkgs/tree/master/manifes...
  - dgellow 4y ago
    Yes, that's always good to check but given the size of the repository (the number of directory and files is just massive!) it can be really annoying to navigate. Often faster to just run the winget CLI somewhere. winget.run should add a link to the manifest directory, that would be useful.

pocholo 4y ago

dcow 4y ago

What is a scenario where you’d be running git in the subdirectory of one owned by a malicious user? Unless a machine is badly configured and administrated, when would one user ever have authority of ownership over /home or /opt or /? And if they have sudo privileges well then they have the authority to do whatever they want. Is this only an issue because of some Windows idiom? I’m somewhat dubious.

dcow 4y ago
I can think of shell prompt plus exploring /tmp, but the fix for this “vuln” doesn't address that issue and seems to be more of a problem with a prompt that automatically runs git in every directory.

fargle 4y ago

It mildly bugs me that things like this are reported as "Git Security Vuln". CVE-12345: insecure use of consumer grade operating system in multi-user role when expecting any form of real isolation CVE-12346: faulty system administration techniques, including running anything as SYSTEM, can cause things to run with elevated privileges CVE-12347: failure to secure root (C:) and important system directories can allow malicious actors to access them. This can be exploited to trick other parts of the system into doing ... things. I don't mind patching git for windows to workaround these things, but sheesh, the root cause of both of these is people using Windows incorrectly/insecurely.

AndrewUnmuted 4y ago
> people using Windows incorrectly/insecurely. Let me fix that for you: > people using Windows.
deleted 4y ago
[deleted]

usrbinbash 4y ago

When people ask me why I don't have a "git-aware" PS1, I shall point them to this CVE in the future.

grumple 4y ago
Would be pretty incredible if the git branch command had a vulnerability. On the other hand, having a git aware PS1 would also immediately alert you to the fact that a user had created a top level .git folder, thereby allowing you to prevent the first cve here.
- usrbinbash 4y ago
  > having a git aware PS1 would also immediately alert you to the fact that a user had created a top level .git folder, And to the fact that someone other than me had write access to my disk, in which case git is probably the least of my worries.

dang 4y ago

All: the originally submitted URL was https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9 https://github.com/git/git/commit/8959555cee7ec045958f9b6dd6.... Readers are divided about which link is better, which probably means you should read both to understand the thread.

Shalomboy 4y ago

There's something super jarring about the format of this blog post. I think my brain has been trained to glaze over whenever it runs into corporate abstract art at the top of a page.

deleted 4y ago

[deleted]

legalcorrection 4y ago

The more I think about it, the more I think this is the right call. The only alternative would be something like falling back to running with no hooks and printing a warning to stderr indicating that there are disabled hooks. Actions that modify that repository should also be disabled in that case. Then there should be a command like 'git hooks trust' that adds the directory to the user's list of trusted folders.

kazinator 4y ago

If you can create a .git directory above a victim's home directory, then you're root. Or else, if you're not root, you're in messed up system. Whoever is root should go read some 40-year-old book on Unix about how it's supposed to be laid out. This is not a genuine security vulnerability; though of course, it's good to fix it. Here is how I would fix it. Forget about permissions and ownership entirely. There is a weaker, more powerful condition we can check. Ready? Git should terminate if it is executed from a subdirectory of a git repo that contains no tracked files according to the first .git/ directory that it finds while ascending the file system. If you're in a directory that contains no files that are tracked by the closest .git/ that can be found by walking up the stairs, then that directory has no relationship to that repo. Git should diagnose that and bail out. (It could allow files in that directory to be added to the index, but only with -f option to force it.) If git finds a .git/ dir, and that repo's index shows that at least one item in, or below, your working directory is in that repo's index, it should go ahead and work with it, regardless of ownership.

xyse53 4y ago
The issue isn't specific to home directories. /tmp, for example. Your suggestion may protect against accidents, but doesn't seem to me to do anything for deliberately malicious behavior.
- kazinator 4y ago
  Right, so someone could create a malicious /tmp/.git. You then go to /tmp/experiment to do something and run some git commands. Easy fix: on boot, have the "rc" script create a root-owned /tmp/.git dummy file with r-------- permissions. Someone can also create a /tmp/foo/.git; but to be susceptible to that, you have to be under /tmp/foo. That's another user's directory. What are you doing in there? Serves you right. If /tmp/foo is your own, and someone planted a .git into it, that's your problem too: you're creating material in /tmp that is accessible to others, which is a security no-no. Probably, this should be fixed in the kernel: the kernel should not allow a regular user to create a hidden directory (i.e. name starting with ".") in /tmp. Or probably any hidden object. Such a fix is more general: it fixes the issue for any git-like program that walks up the tree looking for a special dot directory or file, including all such programs not yet written. The rule could be general, such that creating a hidden object in a directory is only allowed to the directory's owner, not just to anyone who has write permissions to the directory. In other words, if multiple users have write access to a directory, such as /tmp, but any other kind of directory, then they are not allowed to perpetrate hidden objects on each other (both because those things don't show up under "ls" without "-a" and because programs find those and react to them). In fact, I would go one step further and enforce the kernel rule that writing to an existing dot file is denied to anyone other than the owner that file, regardless of its write permissions.

silverwind 4y ago

This "fix" breaks deployments where files are checked out as the root user and then chowned to an app-specific user. Any subsequent action as the root user will fail. It seems they forgot to provide an exception for the root user or a way to disable this "feature" on a global level, instead of per-directory.

deleted 4y ago

[deleted]