Re: REGRESSION: shellshock patch rejects valid function names

[Chet Ramey]

On 9/27/14, 4:29 PM, Eric Blake wrote:
> On 09/27/2014 12:53 PM, Chet Ramey wrote:

> Right now, we know of no way for an attacker to force an arbitrary
> variable name - ONLY arbitrary variable contents.  

Sure, but we didn't know that at the time.  We still don't, really.

> So I would prefer a
> patch that allows the export/import of the exact same set of names as
> what can be used as valid function names.  Neither set should be larger
> than the other, and for back-compat purposes, at least in bash mode, the
> set needs to STILL allow for functions named 'foo::bar' or 'foo/bar'.

Yes, I don't think anyone disagrees that the set of allowed imported
functions should not be different than the set of functions that a shell
allows the user to define.  I am not certain that a shell running in
Posix mode should import a shell function with a name that it wouldn't
let a user define.  That's the point behind the original bash-4.3 check.
The question is what *other* characters to disallow.  I think there's a
good case to be made for including `/', since it can be used to defeat
the usual command lookup order.  There might be others.

> Exactly, this break is undesirable.  Remember, the security hole of
> Shell Shock is NOT what the function is named, but the fact that
> arbitrary variable contents were being parsed.  Once you plug that hole
> by sticking function import/export in a separate namespace, then there
> should be no problem in allowing ALL function names that have always
> been previously allowed.

We have an opportunity to close up a potential problem here, at least
with respect to function names containing `/'.

> I'm 50:50 between two options:
> 
> 1: Have two sets of valid names.  In /bin/sh POSIX mode, functions can
> only be variable names, and when /bin/sh is importing from the env, it
> silently ignores or discards any mangled names that would not fit that
> restriction; meanwhile, invoking a function is only possible if the
> function name is a variable name; then in bash mode, functions can have
> any name and be invoked by that name

That's what the original bash-4.3 code did, with the difference that it
created `normal' shell variables, as would be done for any environment
variable, for names that it wouldn't allow as shell functions.

> 2. Have only a single rule on what forms a valid name.  POSIX doesn't
> forbid us from having an enlarged namespace for valid function names; a
> strictly conforming application won't use such unusual names.  You may
> still want to ignore or scrub those out of the environment during
> /bin/sh importing, but that brings us back to the question of whether
> function imports should be automatic, or something the user has to
> explicitly request.  If ALL function imports are skipped unless
> explicitly requested, then it doesn't matter WHAT function names are
> exported, the incoming functions in a child shell (regardless of whether
> the shell is in bash or /bin/sh mode) will have no impact to that child
> unless the script programmer of the parent asked to have functions imported.

I'm not really interested in the function-importing-by-default discussion
right now.  Maybe sometime later.  It's more of a break with backwards
compatibility than I'm willing to go at this point.

> 
> Thankfully, bash already forbids trying to name a function 'a=b' (having
> to support such a name would make env-var export/import rather
> difficult), so the main culprits that people seem to be worried about
> are '-', '.', ':', and '/'.

The only one of these that really can cause a problem is `/', by
`perverting' (heh) the usual command lookup.  Even a script that thinks
it's protecting itself by using full pathnames can be circumvented by
a function name containing a slash, as in the example I gave.

>> (And I'm not interested in rehashing decisions that were made 25 years
>> ago, and I am completely aware that this "violates" Posix.  That's why it
>> doesn't do this in posix mode.)
> 
> I don't see function export/import as a violation of posix; 

Yeah, but what I was talking about was allowing non-identifiers as valid
function names.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/