Hi Junio
On 05/05/2022 19:33, Junio C Hamano wrote:
Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.
