RE: a problem with git describe
- From
rsbecker@nexbridge.com <rsbecker@nexbridge.com>
- Date
- Apr 26, 2022, 15:56 UTC
- Message-ID
- <049e01d85986$2afeeff0$80fccfd0$@nexbridge.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.2204261742120.355@tvgsbejvaqbjf.bet>
On April 26, 2022 11:44 AM, Johannes Schindelin wrote:
Show 53 quoted lines
>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@gmail.com> 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