Subject: Re: [PATCH v3 1/3] t: document regression git safe.directory when using sudo

From: Junio C Hamano <[email-masked]>
Date: Mon, 09 May 2022 09:01:12 -0700

Phillip Wood <[email-masked]> writes:

>> That way we can avoid having to run "chown" while preparing for the
>> test fixture, and running "git status" under root, but I am not sure
>> if we want our shipped production binaries to have these "pretend"
>> knobs.
>
> Lets ask ourselves "How could an attacker use these knobs to
> facilitate an attack?". They need to either (a) set these variables in
> the user's environment themselves or (b) persuade the user to set
> them.

I actually was not worried about the scenario where an attacker
fools potential victims, who are more privileged than the attacker,
into doing stupid things to hurt themselves.  I mentioned that the
existing knob for testing, "pretend that euid and st_uid are
different", because it tightens the check (even if you are trying
the code with your own directory, euid==st.st_uid check would not
give you an access and you are forced to rely on the safe.directory
cofniguration allowing you in), not loosens it, and it felt much
less risky than "pretend we are root" or "pretend the directory is
owned by root", which just felt much riskier things to allow people
to have us pretend.

But I was totally wrong ;-)  No matter what a unprivileged attacker
does with these knobs, the actual access will be done by a process
run by the attacker, and the actual security at the filesystem level
still kicks in to prevent the attacker from doing anything that the
attacker cannot normally do.  So the only qualm about the proliferation
of these GIT_TEST_* environment variable checks in the production code
is that it makes the logic ugly with more code.

Thanks.