Odd git-config behavior

Hey all,

I recently ran into interesting behavior with git-config (which I originally thought was a bug).

I was converting repos from the deprecated core.fsyncObjectFiles to the new core.fsync option suite; I wrote some automation to do that, using "git config -l" to detect previously-converted repos.

But then some weekly repo maintenance automation complained about the deprecated fsync option being present in some of the repo configs, even though I thought those repos had been converted to the new settings.

I did some digging, and it turns out that "git config -l" was reporting nothing (no output) in Git 2.37.4. I did some testing, and found that Git 2.35.1 correctly reporting the repo's config settings.

Interestingly, the maintenance automation runs fsck and some other things, and reports the presence of the deprecated fsync setting (which is what made me notice); so that code path does read the config and run (and complain about the presence of deprecated settings).

I did a git bisect between 2.35.1 and 2.37.4, and it looks like the following commit changes the behavior:

8959555cee7ec045958f9b6dd62e541affb7e7d9 is the first bad commit
commit 8959555cee7ec045958f9b6dd62e541affb7e7d9
Author: Johannes Schindelin <johannes.schindelin@gmx.de>
Date:   Wed Mar 2 12:23:04 2022 +0100

    setup_git_directory(): add an owner check for the top-level directory
    
    It poses a security risk to search for a git directory outside of the
    directories owned by the current user.

    [full commit message clipped]

So... my maintenance automation runs as root, and the repo directories are uid/gid'd someone else (though, the config file inside the [bare] repo happens to be owned by root)... so I suppose what I'm observing is expected behavior?

I guess this leaves me with two questions:

    1. Why does "git config" refuse to run due to this security check, but
       other git commands ("git fsck," at least) run?

    2. I think it might be useful to warn the user that the behavior they're
       expecting isn't happening due to this security check, instead of just
       outputting objectively wrong information (i.e. that no config options
       exist when they actually do exist); I'd be curious what others think.

best, preed

Hi Paul,

On 2022-10-31 16:51, J. Paul Reed wrote:

You definitively shouldn't run these checks as root. Git is a highly flexible/extensible product and god knows (and likely a few others in this ML too) what could happen while you run git commands as root on an "untrusted" user repository.

What prevents you from getting the owned uid or the repos and forking a process as that user to run the check?

Arguably a read-only config operation could likely be allowed, unless there is a possibility for some untrusted commands to be executed as part of this. I'm going to guess this was an easy entry point to cover dangerous commands, but having a finer-grained check would require making sure there's no dangerous code paths that could be exposed. The change was to address CVE-2022-24765 - commit 8959555cee7 - so there was likely time constraints to consider as well.

What was the return code for the git config command? If it was zero when it didn't parse/output the config option you asked for that is definitively a bug. If you failed to check the return code of git-config then you should fix your script/tool instead.

Regards,

-- Thomas

On 04 Nov 2022 at 07:47:46, Thomas Guyot arranged the bits on my disk to say:

Laziness?

I should note: these aren't really "untrusted" user repositories, so I'm not very concerned about it (though I understand your point).

This does beg the question: does running "git fsck" on an untrusted repository as another user present a [security] problem?

If so, should it?

underworld # ~preed/src/git/git --version git version 2.30.2.4.g8959555cee underworld # GIT_PAGER=cat ~preed/src/git/git-config -l underworld # echo $? 0

best, preed

On 2022-11-08 18:08, J. Paul Reed wrote:

Probably not, but I can't say for sure. Even some seemingly safe commands can be dangerous in this context; for example "git gc --auto" invokes a hook which could execute arbitrary code if run on an untrusted repo.

I haven't read the CVE but did notice the change - the primary issue if I'm not mistaken is when git behaves differently when there is a .git dir that could have been placed by a malicious user. I believe a safe approach has been taken where we have to explicitly whitelist repos or paths where the repos are trusted

We should test with the latest version... If git ignores the config it should warn (like other commands do) and not return 0.

Since git normally uses the global config when not a repo, it appears it keeps looking for the global config after it decides the local one is no good. What you see with this command is your global config not your repo's config.

Regards,

-- Thomas