Re: [PATCH (not final)] (take three?) unset "array[$anything]"

[Stephane Chazelas <stephane@xxxxxxxxxxxx>]

2021-06-04 11:36:09 -0700, Bart Schaefer:
> On Fri, Jun 4, 2021 at 1:02 AM Stephane Chazelas <stephane@xxxxxxxxxxxx> wrote:
> >
> > It also means the lexer, a large and complex bit of code whose
> > behaviour also depends on a the setting of a number of options
> > will end up being exposed to user-supplied data (of the users of
> > the zsh scripts, not just the users of zsh), which adds some
> > level of risk.
> 
> This remark baffles me.  The exact same code is used for ${v/pat/repl}
> among other things, I haven't added any new entry point to the lexer.

But there (and I expect most other contexts where that is used),
the repl is not data supplied by the user of the script, it will
be typically fixed code like $replacement as supplied by the
*author* of the script.

While in:

while IFS=, read -r a b c; do
  unset "hash[$b]"
done < data

And with the stripquotes patch, you'd have the *contents* of $b
(from external data), not the literal $b string fed to the lexer
as if it was meant to be zsh code.

[...]
> > Currently we can do (reliably):
> >
> >    read 'hash[$key]'
> 
> It's unfortunate that this is "reliable" because I'm pretty sure it's
> totally unintentional and should be considered a bug.  "read" should
> not be re-interpreting $key in its NAME parameter.

I'd tend to agree with that statement, and it's this kind of
thing that makes things taking lvalues or arithmetic expressions
very dangerous.

As discussed several times (IIRC, Oliver Kiddle brought it up in
2014 in the aftermaths of shellshock).

  ((x == 1))
  or
  printf %d x
  or
  read "array[x]"

are also command injection vulnerabilities for the same kind of
reason. Here for instance when x='psvar[$(reboot)1]' even
though there's probably not a good reason why $(reboot) would be
expanded when x is referenced in an arithmetic expression.

  
> 
> > and can't do (reliably)
> >
> >    read "hash[$key]"
> 
> That one ought to be reliable if using hash elements here is permitted at all.
> 
> > If we align with whatever solution we pick for unset, we're
> > going to break a lot more scripts. I don't think we can touch
> > those at least in the default mode operation.
> 
> I don't know how many is "a lot" here

I don't have an answer to that either.

> because frankly this is
> something I would never have thought of attempting in the first place
> (particularly, relying on the buggy behavior of the single-quoted
> case).  But I really want to STOP talking about this in the same
> thread as the "unset" question.

Well, it would be good if all those things would be fixed once
and for all and in a consistent way.

There's also the fact that we can't do:

hash['foo bar
baz'$'\n'...]=value

(one needs things like hash[${(pl[1][\n])}]=x to set elements of
key $'\n')

or

array[1 + 1]=4

That is pretty annoying.

Hard to fix without breaking backward portability, but could we
introduce a better design under a new option (setopt
bettersubscriptsandarithmetics), or a "use 6.0" à la perl?

See also https://github.com/ksh93/ksh/issues/152 
where Martijn Dekker has done some improvements on that front in
ksh93.

-- 
Stephane