From: Phillip Wood Date: Thu, 28 Apr 2022 17:56:35 GMT Subject: Re: [PATCH] git-compat-util: avoid failing dir ownership checks if running privileged Message-ID: In-Reply-To: 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