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