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

From: Stephen R. van den Berg <srb@cuci.nl>
Date: Aug 26, 2008, 07:40 UTC
Message-ID: <20080826074044.GA22694@cuci.nl>
In-Reply-To: <48B3A948.3080800@viscovery.net>

Johannes Sixt wrote:

Show 6 quoted lines

>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.
>However, *if* we do this, we must do it for all non-builtins as well!

Well, in general the policy I've used in all the tools I created is that:

a. If it's a setuid tool, then you need to make sure that you don't step
   on anything unintendedly.  I.e. for setuid-something programs this is
   desirable and necessary in order to prevent securityleaks.

b. Anything else is started in an environment controlled by the user,
   and if this environment is broken, then that is the user's fault.
   You get what you wish for.  It's a similar problem you get when you
   set PATH to wrong values and then start "make" for example; it has
   the potential to break a lot; but then again there are infinitely
   more ways to shoot yourself in the foot, than there are ways to
   prevent people from shooting in some particular way.

So I'd say, if the tools are setuid (which none of git's tools are) and are therefore potentially started from a hostile and uncontrolled environment, please make sure filedescriptors 0, 1 and 2 are sane. But for the git utilities, it would be a non-watertight extra safeguard which tries to prevent a situation which rarely occurs and if it does occur, you probably are doing some other things wrong as well; so actually exposing those problems to you by letting you feel the pain can be considered a favour.

-- 
Sincerely,
           Stephen R. van den Berg.

"Good moaning!"