git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Johannes SchindelinNext: Carlo Arenas
Message 13 of 23 in “a problem with git describe”
  1. Guy MaurelApr 23, 2022
  2. Philip OakleyApr 23, 2022
  3. Junio C HamanoApr 23, 2022
  4. Junio C HamanoApr 23, 2022
  5. Carlo Marcelo Arenas BelónApr 25, 2022
  6. SZEDER GáborApr 25, 2022
  7. Carlo Marcelo Arenas BelónApr 25, 2022
  8. Junio C HamanoApr 25, 2022
  9. Carlo Marcelo Arenas BelónApr 25, 2022
  10. Carlo Marcelo Arenas BelónApr 25, 2022
  11. Guy MaurelApr 25, 2022
  12. Johannes SchindelinApr 26, 2022
  13. rsbecker@nexbridge.comApr 26, 2022
  14. Carlo ArenasApr 26, 2022
  15. Junio C HamanoApr 26, 2022
  16. Carlo ArenasApr 26, 2022
  17. Johannes SchindelinApr 26, 2022
  18. Junio C HamanoApr 26, 2022
  19. Taylor BlauApr 26, 2022
  20. Johannes SchindelinApr 26, 2022
  21. Taylor BlauApr 26, 2022
  22. Johannes SchindelinApr 26, 2022
  23. Junio C HamanoApr 26, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.