* Odd git-config behavior
@ 2022-10-31 20:51 J. Paul Reed
2022-11-04 11:47 ` Thomas Guyot
0 siblings, 1 reply; 4+ messages in thread
From: J. Paul Reed @ 2022-10-31 20:51 UTC (permalink / raw)
To: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Odd git-config behavior
2022-10-31 20:51 Odd git-config behavior J. Paul Reed
@ 2022-11-04 11:47 ` Thomas Guyot
2022-11-08 23:08 ` J. Paul Reed
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Guyot @ 2022-11-04 11:47 UTC (permalink / raw)
To: J. Paul Reed, git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Odd git-config behavior
2022-11-04 11:47 ` Thomas Guyot
@ 2022-11-08 23:08 ` J. Paul Reed
2022-11-09 7:02 ` Thomas Guyot
0 siblings, 1 reply; 4+ messages in thread
From: J. Paul Reed @ 2022-11-08 23:08 UTC (permalink / raw)
To: Thomas Guyot; +Cc: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Odd git-config behavior
2022-11-08 23:08 ` J. Paul Reed
@ 2022-11-09 7:02 ` Thomas Guyot
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Guyot @ 2022-11-09 7:02 UTC (permalink / raw)
To: J. Paul Reed; +Cc: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-09 7:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-31 20:51 Odd git-config behavior J. Paul Reed
2022-11-04 11:47 ` Thomas Guyot
2022-11-08 23:08 ` J. Paul Reed
2022-11-09 7:02 ` Thomas Guyot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).