Re: Arithmetic + array allows for code injection

[Maarten Billemont]

On Jun 2, 2014, at 9:34 AM, Greg Wooledge <wooledg@eeg.ccf.org> wrote:

> On Mon, Jun 02, 2014 at 03:08:17PM +0200, Andreas Schwab wrote:
>> Greg Wooledge <wooledg@eeg.ccf.org> writes:
>> 
>>> imadev:~$ : $((a[$x]))
>>> bash: Mon Jun 2 08:06:39 EDT 2014: syntax error in expression (error token 
>>> is "Jun 2 08:06:39 EDT 2014")
>>> 
>>> There's the code-injection problem that started the thread.
>> 
>> Here the index is '$(date)'.
>> 
>> *Note (bash) Arithmetic Expansion:: ... All tokens in the expression
>> undergo parameter and variable expansion, command substitution, and
>> quote removal.  The result is treated as the arithmetic expression to be
>> evaluated.
> 
> Ah.  And this is copied almost verbatim from POSIX:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04
> 
> $((expression))
> 
> The expression shall be treated as if it were in double-quotes, except that a
> double-quote inside the expression is not treated specially. The shell shall
> expand all tokens in the expression for parameter expansion, command
> substitution, and quote removal.
> 
> So the reason it acts this way is because POSIX said so.  Now it starts
> to make sense!
> 
> (One could argue that POSIX's wording doesn't require the command
> substitution be done in a second pass AFTER the parameter expansion.
> But apparently it has been interpreted this way.)

As such, the bug is that the expansions triggered by $(( )) are IMPLICITLY 
re-evaluated by [ ].

To summarize,

index='$( echo >&2 foo )' # Literal shell code should never be evaluated unless 
an 'eval' is involved.
echo ${array[ $index ]} # [] expands $index, results in a literal that [] does 
not re-evaluate.
echo $(( $index )) # (( )) expands $index, results in a literal that (( )) does 
not re-evaluate.
echo $(( array[ $index ] )) # (( )) expands $index, results in a literal that 
[] DOES re-evaluate.

Whether or not the documentation justifies the above behaviour, I think we can 
agree on the fact that no user will expect or even desire the behaviour of the 
latter, never mind that it is dangerous.  As such, it certainly is a bug and 
should be corrected.

I think it's fair to trust, as an author, that shell code will only be 
re-evaluated when there's an explicit eval, source, or similar statement 
declaring such behaviour.
As a side note, expanding literal words as variables such as happens in 
${array[ index ]} or the above code with index=foo is probably acceptable so 
long as the value that results from this expansion is treated purely as data 
(though bash does allow recursion on the expansion of literal variable names 
here).  The point being that in my opinion, bash should as a matter of 
principle prohibit concealed code execution wherever possible; failing that it 
will be nearly impossible to write safe shell code - or at least trust that 
your shell code is safe.

On Jun 2, 2014, at 10:28 AM, Andreas Schwab <schwab@suse.de> wrote:

> If you want to write robust scripts, don't use shell.
> 
> Andreas.

What does this comment justify?  Other than stopping all maintenance on bash 
today.