Odd git-config behavior

Odd git-config behavior

[J. Paul Reed]

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
-- 
J. Paul Reed
https://jpaulreed.com
PGP: 0x41AA0EF1

Re: Odd git-config behavior

[Thomas Guyot]

Hi Paul,

On 2022-10-31 16:51, J. Paul Reed wrote:
> 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?

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?

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

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.

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

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

Re: Odd git-config behavior

[J. Paul Reed]

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

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

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?

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

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
-- 
J. Paul Reed
https://jpaulreed.com
PGP: 0x41AA0EF1

Re: Odd git-config behavior

[Thomas Guyot]

On 2022-11-08 18:08, J. Paul Reed wrote:
> This does beg the question: does running "git fsck" on an untrusted
> repository as another user present a [security] problem?
>
> If so, should it?

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

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