[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: REGRESSION: shellshock patch rejects valid function names

From: Eric Blake
Subject: Re: REGRESSION: shellshock patch rejects valid function names
Date: Sat, 27 Sep 2014 15:23:27 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0

On 09/27/2014 03:10 PM, Jay Freeman (saurik) wrote:
> [I am terribly sorry that my In-Reply-To is wrong :/]
> ----- "Eric Blake" <address@hidden> wrote:
>> ... Remember, the security hole of
>> Shell Shock is NOT what the function is named, but the fact that
>> arbitrary variable contents were being parsed. ...
> Not quite: the point of exploit can be in the variable name.

That is NOT the same exploit as Shell Shock.  It is indeed a
(local-only) issue, that we should be careful to avoid, but the point of
Shell Shock is that attackers were able to influence contents, but NOT
names, of data stuck into the environment.

> $ env 'date;x=() { :;}' bash --norc -c :
> Sat Sep 27 20:40:41 UTC 2014
> Segmentation fault (core dumped)

Yes, I agree that bash should be robust against garbage in the
environment.  And that's true whether or not we stick with the 4.2.26
behavior of (ab)using 'x=() {...}' for function imports, or go with
Florian's patch to use 'BASH_FUNC_x()=() {...}'.  Thus, we want to make
sure that even with Florian's patch, 'BASH_FUNC_date;x()=() {...}' does
NOT case an arbitrary execution of 'date'.  But, as you pointed out, it
is NOT possible for bash to directly export a bogus function name
containing metacharacters, and therefore, the only way to inject
metacharacters into a name that might cause the parser to invoke a
command is via the local environment outside of bash, whereas Shell
Shock is about the problem of arbitrary contents of REGULAR variables on
export being misinterpreted on input.

> While AFAIK you can't create functions with names that eventually lead to 
> dangerous variable names, this is due to a quoting requirement (to place that 
> ";", for example, in a function definition statement, I'd have to quote it, 
> and check_identifier refuses to allow any quoted characters) that happens 
> when parsing the function as it is created "in the first place" that are not 
> quite so easily replicated when trying to use check_identifier to validate 
> the name before importing the function (hence why, even with the patch I 
> provided yesterday that verifies function names before importing using a 
> mechanism similar to that used for definitions, I still rely on SEVAL_FUNCDEF 
> to catch "date;x": the ; has not been "quoted", so check_identifier considers 
> it not to be a problem; incidentally, this is why my original patch concept, 
> before I understood the new SEVAL_FUNCDEF and verified it was sufficient as 
> in the patch I provided yesterday, involved running the name itself through 
> the parser t
o verify it was a single word).

It should be fairly easy to validate whether an incoming environment
name=value pair names a valid shell function name, without having to
resort to the full power (and possible unknown bugs) of the parser.  And
it is only common sense that we do so.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]