Re: [PATCH] be paranoid about closed stdin/stdout/stderr

[Junio C Hamano <gitster@xxxxxxxxx>]

Junio C Hamano <gitster@xxxxxxxxx> writes:

> Paolo Bonzini <bonzini@xxxxxxx> writes:
>
>> Junio C Hamano wrote:
>>> Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:
>>> 
>>>> Paolo Bonzini schrieb:
>>>>> +	/*
>>>>> +	 * Always open file descriptors 0/1/2 to avoid clobbering files
>>>>> +	 * in die().  It also avoids not messing up when the pipes are
>>>>> +	 * dup'ed onto stdin/stdout/stderr in the child processes we spawn.
>>>>> +	 */
>>>> I see your point, but I don't have an opinion whether this stretch is
>>>> necessary.
>>> 
>>> This is going too far.  Have you seen any other sane program that do this?
>>
>> Busybox.  But it runs setuid, as Steven pointed out.
>>
>> I say it's all (i.e. be this paranoid), or nothing.
>
> I tend to agree, and I think what Stephen R. van den Berg said earlier in
> the thread makes perfect sense.

So going back to the very original in the thread.

I think

	$ git fetch 0<&-

from the command line is a mere user stupidity.

On the other hand, if a cron/at job that contains "git fetch" is launched
in an environment with fd#0 (or #1 or #2 for that matter) closed, it would
certainly be problematic.  It can easily be worked around by redirecting
file descriptors appropriately in the script that is launched, though.

On a related note, we should make sure that we run our hooks with the set
of low file descriptors opened sensibly.  It would be a bug if we are
running them in a weird environment and forcing them to do funky
redirection themselves.  I think we are already Ok in this regard, but I
didn't check.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html