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

[Phillip Wood <phillip.wood123@gmail.com>]

On 27/04/2022 19:54, Junio C Hamano wrote:

[ I've trimmed your discussion for brevity but thanks for taking the 
time to write it out, it was helpful in clarifying what we're trying to 
protect ourselves against]

> So, in short, I think it is reasonable if we did:
> 
>   - do the extra "if SUDO_UID exists, then..." only when we are root.
> 
>   - use strtol to ensure SUDO_UID is in good shape (all digits) and
>     read into long; use errno=0 trick to notice overflows.  err on
>     the safe side and do not allow if the value cannot be read
>     correctly.
> 
>   - do the comparison between st.st_uid and strtol() result by
>     casting both to (long).  Without this, st.st_uid has a value that
>     needs wider than long, we cannot allow the owner of such a
>     repository in (and I somehow feel that is unlikely to happen).
> 
>   - or omit the third one --- tough luck for those whose UID is
>     beyond what a long can represent (and I somehow feel that this is
>     probably OK).

I think that unsigned long should be wide enough. On x86 linux uid_t is 
uint32_t for both 32 & 64 bit processors. That means that we could 
truncate the value we read from SUDO_UID when long is 64 bits if it is 
cast to uid_t but we're supposed to be able to trust that SUDO_UID is a 
valid id so that shouldn't be a problem. If an attacker can change 
what's in SUDO_UID we're doomed anyway.

Best Wishes

Phillip