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

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

Hi Carlo

On 09/05/2022 15:51, Carlo Arenas wrote:
> On Mon, May 9, 2022 at 1:21 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> 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.
>>>
>>> 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?".
> 
> That is not the question raised by having those "pretend" knobs in the
> production binary, but instead how can an attacker abuse them to get
> themself and UID he doesn't have and therefore additional access.

Maybe I'm missing something but I thought the idea was that these knobs 
were only for the safe.directory check and the normal file permissions 
would apply to all the other code.

Best Wishes

Phillip

> The fact that the current code requires you to be root to even enable
> the logic makes it more difficult to use SUDO_UID that way, because if
> you already got root, you don't really need them, but take into
> consideration that this discussion starts with (how can we run these
> things as a the test user and avoid sudo, hence root).
> 
> Carlo