Re: [PATCH v3 1/3] t: document regression git safe.directory when using sudo

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

Hi Junio

On 05/05/2022 19:33, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> Hmm. I would like to suggest that we can side-step all of these issues
>> (and the ones I outline below) by considering a similar approach to the
>> one Stolee took in t0033: use one or more `GIT_TEST_*` environment
>> variables to pretend the exact scenario we want to test for.
> 
> I like the GIT_TEST_ASSUME_DIFFERENT_OWNER because it is fairly
> clear that it cannot be used as a new attack vector, even with
> social engineering.
> 
> It would be nice if we can do something similar, but I am coming up
> empty while trying to think of "we are testing; pretend that ..."
> that is good for testing this SUDO_UID special case *and* clearly
> cannot be used as an attack target.
> 
> So I very much like the suggestion in principle, but I am not sure
> how useful the suggestion would be to make the resulting code better
> in practice.
> 
> Perhaps this may be a way to pretend we are running a command under
> 'sudo'?
> 
> 	test_pretend_sudo () {	
>              GIT_TEST_PRETEND_GETEUID_RETURNING_ROOT=1 \
> 	    GIT_TEST_PRETEND_LSTAT_RETURNING_ROOT=root/p \
>                  SUDO_UID=0 "$@"
> 	}
> 
> 	test_expect_success 'access root-owned repository as root' '
> 		mkdir root/p &&
> 		git init root/p &&
> 		test_pretend_sudo git status
> 	'
> 
> That way we can avoid having to run "chown" while preparing for the
> test fixture, and running "git status" under root, but I am not sure
> if we want our shipped production binaries to have these "pretend"
> knobs.

Lets ask ourselves "How could an attacker use these knobs to facilitate 
an attack?". They need to either (a) set these variables in the user's 
environment themselves or (b) persuade the user to set them. For (a) if 
an attacker can set them in the user's environment then they can change 
the user's $PATH or $GIT_DIR and $GIT_WORK_TREE so this does not open up 
a new way to compromise the user. For (b) I'm not sure it is easier to 
socially engineer the user to set these new variables rather than 
GIT_DIR and GIT_WORK_TREE which will also bypass the check.

Best Wishes

Phillip


> Thanks.