On April 26, 2022 11:44 AM, Johannes Schindelin wrote:
>On Mon, 25 Apr 2022, Carlo Marcelo Arenas Belón wrote:
>
>> On Mon, Apr 25, 2022 at 12:02:45AM -0700, Carlo Marcelo Arenas Belón wrote:
>> > On Sun, Apr 24, 2022 at 11:39:27PM -0700, Junio C Hamano wrote:
>> > > Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes:
>> > >
>> > > > At that point, though you might as well excempt root from this
>> > > > check
>> > >
>> > > But "root" or any higher-valued account is what needs this kind of
>> > > protection the most, no?
>> >
>> > correct, and I didn't meant to excempt root from the protection, but
>> > from the check that requires that the config file ownership matches.
>> >
>> > if the config file is owned by root, we already lost, regardless of
>> > what uid git is running as.
>>
>> apologies for my confusing english, hopefully this C is clearer
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h index
>> 58fd813bd01..6a385be7d1d 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -440,9 +440,19 @@ static inline int git_offset_1st_component(const
>> char *path) static inline int is_path_owned_by_current_uid(const char
>> *path) {
>> struct stat st;
>> + uid_t euid;
>> +
>> if (lstat(path, &st))
>> return 0;
>> - return st.st_uid == geteuid();
>> +
>> + euid = geteuid();
>> + if (!euid && st.st_uid && isatty(0)) {
>> + struct stat ttyst;
>> + if (!stat(ttyname(0), &ttyst))
>> + euid = ttyst.st_uid;
>> + }
>> +
>> + return st.st_uid == euid;
>> }
>>
>> #define is_path_owned_by_current_user is_path_owned_by_current_uid
>>
>> it uses stdin instead not to fall in the issue that was raised by
>> Gábor, but I am affraid that it might need to check all stdnandles for
>> a valid tty to be safe, and it looking even more complex.
>
>Maybe a better idea for the `sudo` scenario would be to make use of `SUDO_UID`
>(assuming that no adversary can gain control over the user's environment
>variables)?
Please do not assume that the user id 0 represents root. It does not on all systems. This change will cause breakages. We would need a specific comparison in compat.c with platform-specific values.
For example:
#if defined __TANDEM
#define ROOT_UID 65535
#define ROOT_GID 255
#endif
--Randall
