Re: Issues with exported functions

[Ángel González]

On Eric Blake wrote:
> On 09/28/2014 10:31 AM, Ángel González wrote:
> > David A. Wheeler wrote:
> >> 2. Import environment variables *ONLY* when they are requested; do *NOT* 
> >> import them by default.  Christos Zoulas has proposed this.  This *IS* a 
> >> real backwards-incompatible change.  But most users do *NOT* use this 
> >> functionality, and increasingly downstream systems are *already* switching 
> >> to this mode.  E.G., FreeBSD has already switched to this; function 
> >> imports require --import-functions or enabling the IMPORTFUNCTIONS option. 
> >>   E.G., see: https://svnweb.freebsd.org/ports?view=revision&revision=369341
> >> This change eliminates the entire class of problems.  It's still good to 
> >> do #1, even with #2, because if someone DOES perform an import, it reduces 
> >> the probability of accidentally importing the wrong thing.  People are 
> >> ALREADY making this change, whether upstream does or not.
> >>
> > 
> > There's also the middleground of not parsing the environment variables
> > before they are going to be used. That avoids the issues caused by
> > parsing what is not needed *and* doesn't break backwards compatibility.
> > See the patch I sent a couple days ago.
> 
> That patch doesn't address the fact that variables and functions are in
> different namespaces, though.  That is, if I do 'export f=1; f(){ echo
> 2;}; export -f f', then what would "$f" be in the child shell? 

It would be (and is) 1, as required.


> With just your patch, but not the fix for separate namespaces of 4.3.27, you
> are still trying to pass two separate 'f=...' environment variables,

That's right, exactly what bash < 4.3.27 did.


> with unspecified results, and risk the child getting 'echo "$f"' to see
> the unexpected "() { echo 2; }' instead of the expected "1".  

The bash children correctly get the two things, there are no unspecified
results for them.

The problems are:
* Non-bash programs reading f from the environment
* bash programs reading a free-text variable from the environment (which
the attacker can prefix with "() {")


> Your approach of lazy parsing may still have benefits, but it is not a middle
> ground, in that we MUST have separate namespaces (patch 27), 

The reasons I listed above justify having separate namespaces. The only
concern not to do it was BC. I agree with patch 27


> and once we have separate namespaces, your patch is no longer adding 
> security, just
> optimization.

No. If we had a perfect bash with no parser bugs (which we won't be able
to prove to have reached), then my patch would only be an optmization.
Otherwise  it does add a security layer over that of patch 27 -although
the BASH_FUNCT prefix already sets the bar so high it might be
impossible to bypass-. 



> >> John Haxby recently posted that "A friend of mine said this could be a 
> >> vulnerability gift that keeps on giving." 
> >> (http://seclists.org/oss-sec/2014/q3/748).  Bash will be a continuous rich 
> >> source of system vulnerabilities until it STOPS automatically parsing 
> >> normal environment variables; all other shells just pass them through!   
> >> I've turned off several websites I control because I have *no* confidence 
> >> that the current official bash patches actually stop anyone, and I am 
> >> deliberately *not* buying products online today for the same reason.  I 
> >> suspect others have done the same.  I think it's important that bash 
> >> change its semantics so that it "obviously has absolutely no problems of 
> >> this kind".
> > 
> > That's exactly what my patch does, although it wouldn't be transparent
> > if used inside bash (eg. echo $FUNC), as opposed of usage by its
> > children (wouldn't be hard to change, though).
> 
> I consider it an important design goal to ensure that ALL exported
> variables cannot be corrupted no matter what their contents are.  We
> aren't quite there yet (due to the issues of 'function a=b () {:;}
> corrupting the variable named BASH_FUNC_a even after patch 27 is
> applied).

Using the colon there makes it invalid. Pasting the poc from the other
thread: 
> env -i bash -c 'function a=b(){ echo oops;};export -f a=b;export 
> BASH_FUNC_a=hi; bash -c "echo \$BASH_FUNC_a"'



> But your own admission that $FUNC may be corrupted is an argument against 
> your patch in isolation.

Fair enough, I acknowledged from the beginning that it could be
combined with the other flying patches. I was trying to do only one fix.
Actually, I was thinking that the  move should be to use a single
namespace. I realized now that it is a POSIX requirement that they are
separate:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_05

Regards