Re: [PATCH v3 2/3] git-compat-util: avoid failing dir ownership checks if running privileged

[Carlo Arenas <carenas@gmail.com>]

On Fri, May 6, 2022 at 1:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Carlo Arenas <carenas@gmail.com> writes:
>
> > which is also why we can't use it, any possibly bogus or suspicious
> > value we get from SUDO_UID MUST be ignored.
>
> I do not think I agree.  If we have a strange value in SUDO_UID, it
> would be much better and safer to err on the safe side.

ignoring it is the safe side; for example if we replace the current
function with the proposed one then some user lucky enough to have
access to the latest linux supercomputer that has been patched to have
a 64-bit uid_t (because who makes 32-bit supercomputers nowadays)
would get root[1] access by simply faking his SUDO_UID to be UINT_MAX
+ 1.

We will also honour probably SUDO_UID=0M as root instead of the
current action which is to ignore that nonsense and most likely die by
telling the pranker that he still can't run `git status` on that root
owned repository he got access to even after he managed to get sudo to
generate that as a SUDO_UID.

> Instead of ignoring, in the situation where we care about the value
> we read from SUDO_UID (i.e. when euid==0), we should die loudly when
> it has a strange value.

that is fair, but then it would then make this feature into a denial
of service attack target ;)

The current implementation instead keeps git running under the UID it
was started as, which should be root if it gets to use this code under
the current implementation.

I am still open to changing it if you would rather let git be the last
line of defense, I just think that the current implementation of
ignoring it is more user friendly and better at punking would be
attackers.

Carlo

[1] https://lwn.net/Articles/727490/