Re: REGRESSION: shellshock patch rejects valid function names

[Jay Freeman (saurik)]

----- "Ángel González" <address@hidden> wrote:

> The patch seems straightforward:
> 
> diff --git a/variables.c b/variables.c
> index 92a5a10..6552e69 100644
> --- a/variables.c
> +++ b/variables.c
> @@ -361,7 +361,7 @@ initialize_shell_variables (env, privmode)
...
> -         if (legal_identifier (name))
> +         if (check_identifier (name))
...

This patch would not work (it would not even compile if you tried it, in fact 
;P) because check_identifier takes two arguments: the second argument is 
whether to internally run legal_identifier on the name. Additionally, 
check_argument takes a WORD_DESC, not a string.

If these two issues were fixed (by using make_word or something, and then 
passing 0 or preferably posixly_correct), I am concerned that this might 
undermine the security fix itself, as check_identifier could potentially allow 
ludicrous things to be pasted in the name?

It seems, however, like SEVAL_FUNCDEF is sufficient to keep most things that 
should not happen from actually happening? I tested the attached patch against 
the following test case. It might be that I went overboard and don't actually 
need quote_escapes (I'm new to bash).

(function std:echo() { echo "$@"; }; export -f std:echo; env 'x$(date)=() { 
:;}' 'x`date`=() { :;}' 'date;x=() { date;}' ./bash --norc -c 'std:echo hello 
world')

./bash: `x$(date)': not a valid identifier
./bash: error importing function definition for `x$(date)'
./bash: `x`date`': not a valid identifier
./bash: error importing function definition for `x`date`'
./bash: warning: date;x: ignoring function definition attempt
./bash: error importing function definition for `date;x'
hello world

colon-functions-attempt-1.diff
Description: Binary data