[osh-language] Implement new eval_unsafe_arith on unset, printf, ${!ref}

This is related to the fix for #1450.  It relaxes eval_unsafe_arith in
the remaining places.

One more test in spec/nix-idioms now fails.  It passed for the wrong
reason -- it because 'foo[@]' wasn't a valid var ref, not because we
implemented the weird and arguably buggy "empty array + var ref" rule
that bash has.

Also:

- Remove eval_unsafe_arith from almost all spec tests.
- Update docs.

core/shell.py

@@ -444,8 +444,8 @@ def Main(lang, arg_r, environ, login_shell, loader, readline):
   # Initialize builtins that depend on evaluators
   #
 
-  unsafe_arith = sh_expr_eval.UnsafeArith(mem, exec_opts, parse_ctx, arith_ev,
-                                          errfmt)
+  unsafe_arith = sh_expr_eval.UnsafeArith(mem, exec_opts, mutable_opts,
+                                          parse_ctx, arith_ev, errfmt)
   vm.InitUnsafeArith(mem, word_ev, unsafe_arith)
 
   builtins[builtin_i.printf] = builtin_printf.Printf(mem, parse_ctx,

doc/known-differences.md

@@ -30,37 +30,31 @@ TODO:
 
 ## Numbers and Arithmetic
 
-Roughly speaking, shells treat arithmetic like "macro processing", while OSH
-treats it more like part of a programming language.
-
-Despite these differences, OSH is very compatible with existing shell scripts.
-
-Note that you can opt into more errors with `shopt -s strict_arith`.
-
-### Static Parsing
+### printf '%d' and other numeric formats require a valid integer
 
-Arithmetic is [statically parsed](https://www.oilshell.org/blog/2016/10/22.html), so expressions like `$(( 1 $op 2 ))` fail with
-a parse error.  Use an explicit `eval` for these rare use cases.
+In other shells, `printf %d invalid_integer` prints `0` and a warning.  OSH
+gives you a runtime error.
 
-Related: [A 30-year-old security problem](https://www.oilshell.org/blog/2019/01/18.html#a-story-about-a-30-year-old-security-problem) / [Simple Word Evaluation](simple-word-eval.html)
+### Code Parsed from Data Can't Have Command Subs unless `shopt -s eval_unsafe_arith`
 
-### Array Indices Are Static unless `shopt -s eval_unsafe_arith`
+In shell, array locations are often dynamically parsed, and the index can have
+command subs, which execute arbitrary code.
 
-If you have a variable like `code='1+2'`, OSH doesn't accept
+For example, if you have `code='a[$(echo 42 | tee PWNED)]'`, shells will parse
+this data and execute it in many sitautions:
 
-    a[$code]=value  # dynamic parsing and evaluation in bash, mksh, zsh
+    echo $(( code ))  # dynamic parsing and evaluation in bash, mksh, zsh
 
-or
+    unset $code
 
-    echo ${a[$code]}  # ditto
+    printf -v $code hi
 
-by default.  If you want this behavior, you can turn on `shopt -s
-eval_unsafe_arith`.
+    echo ${!code}
 
-### printf '%d' and other numeric formats require a valid integer
+OSH disallows this by default.  If you want this behavior, you can turn on
+`shopt -s eval_unsafe_arith`.
 
-In other shells, `printf %d invalid_integer` prints `0` and a warning.  OSH
-gives you a runtime error.
+Related: [A 30-year-old security problem](https://www.oilshell.org/blog/2019/01/18.html#a-story-about-a-30-year-old-security-problem)
 
 ## Parsing Differences
 

doc/oil-help-topics.md

@@ -190,11 +190,12 @@ X [External Lang] BEGIN   END   when (awk)
                   X old_syntax (-u)      [[   $(( ))  ${x%prefix}   ${a[@]}
                                          $$
   [Compatibility] compat_array           ${array} is ${array[0]}
-                  eval_unsafe_arith      Recursively parse and evaluate
+                  eval_unsafe_arith      Allow dynamically parsed a[$(echo 42)]
                   parse_dynamic_arith    LHS can contain variables
                   verbose_errexit        Whether to print detailed errors
-  [More Options]  allow_csub_psub        For implementing strict_errexit
-                  dynamic_scope          For implementing 'proc'
+  [More Options]  _allow_command_sub     To implement strict_errexit, eval_unsafe_arith
+                  _allow_process_sub     To implement strict_errexit
+                  dynamic_scope          To implement 'proc'
 ```
 
 <h2 id="env">

osh/sh_expr_eval.py

@@ -184,10 +184,11 @@ def IsUpper(ch):
 class UnsafeArith(object):
   """For parsing a[i] at RUNTIME."""
 
-  def __init__(self, mem, exec_opts, parse_ctx, arith_ev, errfmt):
-    # type: (state.Mem, optview.Exec, parse_lib.ParseContext, ArithEvaluator, ui.ErrorFormatter) -> None
+  def __init__(self, mem, exec_opts, mutable_opts, parse_ctx, arith_ev, errfmt):
+    # type: (state.Mem, optview.Exec, state.MutableOpts, parse_lib.ParseContext, ArithEvaluator, ui.ErrorFormatter) -> None
     self.mem = mem
     self.exec_opts = exec_opts
+    self.mutable_opts = mutable_opts
     self.parse_ctx = parse_ctx
     self.arith_ev = arith_ev
     self.errfmt = errfmt
@@ -210,14 +211,14 @@ def ParseLValue(self, s, span_id):
         # Exception for builtins 'unset' and 'printf'
         e_usage('got invalid place expression', span_id=span_id)
 
-    lval = self.arith_ev.EvalArithLhs(anode, span_id)
-
-    # Prevent attacks like these by default:
-    #
-    # unset -v 'A["$(echo K; rm *)"]'
-    if not self.exec_opts.eval_unsafe_arith() and lval.tag_() != lvalue_e.Named:
-      e_usage('expected a var name.  shopt -s eval_unsafe_arith allows a[i]',
-              span_id=span_id)
+    if self.exec_opts.eval_unsafe_arith():
+      lval = self.arith_ev.EvalArithLhs(anode, span_id)
+    else:
+      # Prevent attacks like these by default:
+      #
+      # unset -v 'A["$(echo K; rm *)"]'
+      with state.ctx_Option(self.mutable_opts, [option_i._allow_command_sub], False):
+        lval = self.arith_ev.EvalArithLhs(anode, span_id)
 
     return lval
 

osh/word_eval.py

@@ -27,6 +27,7 @@
     a_index, a_index_e, a_index__Int, a_index__Str,
     VTestPlace, VarSubState,
 )
+from _devbuild.gen.option_asdl import option_i
 from asdl import runtime
 from core import error
 from core import pyos
@@ -758,8 +759,6 @@ def _EvalVarRef(self, val, blame_tok, quoted, vsub_state, vtest_place):
       elif case(value_e.Str):
         val = cast(value__Str, UP_val)
         bvs_part = self.unsafe_arith.ParseVarRef(val.s, blame_tok)
-        if not self.exec_opts.eval_unsafe_arith() and bvs_part.bracket_op:
-          e_die('a[i] not allowed without shopt -s eval_unsafe_arith', blame_tok)
         return self._VarRefValue(bvs_part, quoted, vsub_state, vtest_place)
 
       elif case(value_e.MaybeStrArray):  # caught earlier but OK
@@ -1160,7 +1159,12 @@ def _VarRefValue(self, part, quoted, vsub_state, vtest_place):
       val = self._EvalSpecialVar(part.token.id, quoted, vsub_state)
 
     # We don't need var_index because it's only for L-Values of test ops?
-    val = self._EvalBracketOp(val, part, quoted, vsub_state, vtest_place)
+    if self.exec_opts.eval_unsafe_arith():
+      val = self._EvalBracketOp(val, part, quoted, vsub_state, vtest_place)
+    else:
+      with state.ctx_Option(self.mutable_opts, [option_i._allow_command_sub], False):
+        val = self._EvalBracketOp(val, part, quoted, vsub_state, vtest_place)
+
     return val
 
   def _EvalBracedVarSub(self, part, part_vals, quoted):

spec/arith.test.sh

@@ -543,8 +543,7 @@ xbar[5]=42
 ## N-I dash status: 2
 ## N-I dash stdout-json: ""
 
-#### shopt -s eval_unsafe_arith
-shopt -s eval_unsafe_arith
+#### Dynamic parsing of arithmetic
 e=1+2
 echo $(( e + 3 ))
 [[ e -eq 3 ]] && echo true
@@ -563,8 +562,7 @@ status=0
 ## N-I dash status: 2
 ## N-I dash stdout-json: ""
 
-#### eval_unsafe_arith on empty string
-shopt -s eval_unsafe_arith
+#### Dynamic parsing on empty string
 a=''
 echo $(( a ))
 
@@ -605,7 +603,6 @@ $SH -c 'echo $((a + 42x))'
 echo status=$?
 
 # regression
-shopt -s eval_unsafe_arith
 echo $((a + 42x))
 echo status=$?
 ## status: 1

spec/array.test.sh

@@ -108,7 +108,6 @@ argv.py "${a[-1]}" "${a[-2]}" "${a[-5]}"  # last one out of bounds
 ## N-I mksh stdout: ['', '', '']
 
 #### Negative index and sparse array
-shopt -s eval_unsafe_arith  # for unset
 a=(0 1 2 3 4)
 unset a[1]
 unset a[4]
@@ -146,7 +145,6 @@ echo ${a[@]}
 ## END
 
 #### Negative index and sparse array
-shopt -s eval_unsafe_arith  # for unset
 a=(0 1)
 unset 'a[-1]'  # remove last element
 a+=(2 3)
@@ -168,7 +166,6 @@ echo ${a[3]} $((a[3]))
 ## END
 
 #### Length after unset
-shopt -s eval_unsafe_arith
 a=(0 1 2 3)
 unset a[-1]
 echo len=${#a[@]}
@@ -591,8 +588,7 @@ foo
 ## END
 
 
-#### Dynamic parsing of LHS a[$code]=value (eval_unsafe_arith)
-shopt -s eval_unsafe_arith  # 
+#### Dynamic parsing of LHS a[$code]=value
 
 declare -a array
 array[x=1]='one'
@@ -613,9 +609,7 @@ y=2
 ## N-I dash stdout-json: ""
 ## N-I dash status: 2
 
-#### Dynamic parsing of RHS ${a[$code]} (eval_unsafe_arith)
-shopt -s eval_unsafe_arith
-
+#### Dynamic parsing of RHS ${a[$code]}
 declare -a array
 array=(zero one two three)
 

spec/assoc.test.sh

@@ -543,7 +543,6 @@ values: val3
 ## END
 
 #### ${!ref} and assoc array
-shopt -s eval_unsafe_arith
 
 show-values() {
   echo values: ${A[@]}
@@ -566,7 +565,6 @@ ref val
 ## END
 
 #### printf -v and assoc array
-shopt -s eval_unsafe_arith
 
 show-values() {
   echo values: ${assoc[@]}

spec/bugs.test.sh

@@ -241,10 +241,7 @@ esac
 NOT SPLIT
 ## END
 
-#### autoconf arithmetic - eval_unsafe_arith (#1450)
-
-# Fixes the bug, but not sure
-# shopt -s eval_unsafe_arith
+#### autoconf arithmetic - relaxed eval_unsafe_arith (#1450)
 
 as_fn_arith ()
 {
@@ -303,3 +300,44 @@ fi
 NOPE
 ## END
 
+
+#### unset doesn't allow command execution
+
+typeset -a a  # for mksh
+a=(42)
+echo len=${#a[@]}
+
+unset -v 'a[$(echo 0 | tee PWNED)]'
+echo len=${#a[@]}
+
+if test -f PWNED; then
+  echo PWNED
+  cat PWNED
+else
+  echo NOPE
+fi
+
+## status: 1
+## STDOUT:
+len=1
+## END
+
+## N-I dash/ash status: 2
+## N-I dash/ash stdout-json: ""
+
+## BUG bash/mksh status: 0
+## BUG bash/mksh STDOUT:
+len=1
+len=0
+PWNED
+0
+## END
+
+## BUG zsh status: 0
+## BUG zsh STDOUT:
+len=1
+len=1
+PWNED
+0
+## END
+

spec/builtin-printf.test.sh

@@ -55,7 +55,6 @@ OK
 ## N-I dash status: 1
 
 #### printf -v a[1]
-shopt -s eval_unsafe_arith
 a=(a b c)
 printf -v 'a[1]' %s 'foo'
 echo status=$?
@@ -72,7 +71,6 @@ status=0
 ## N-I dash/ash status: 2
 
 #### printf -v syntax error
-shopt -s eval_unsafe_arith
 printf -v 'a[' %s 'foo'
 echo status=$?
 ## STDOUT:

spec/builtin-vars.test.sh

@@ -440,8 +440,6 @@ status=127
 ## END
 
 #### Unset array member
-shopt -s eval_unsafe_arith
-
 a=(x y z)
 unset 'a[1]'
 echo status=$?
@@ -458,8 +456,6 @@ status=0
 ## END
 
 #### Unset errors
-shopt -s eval_unsafe_arith
-
 unset undef
 echo status=$?
 
@@ -483,8 +479,6 @@ status=0
 #### Unset wrong type
 case $SH in (mksh) exit ;; esac
 
-shopt -s eval_unsafe_arith || true
-
 declare undef
 unset -v 'undef[1]'
 echo undef $?
@@ -536,7 +530,6 @@ assoc 0
 
 
 #### unset -v assoc (related to issue #661)
-shopt -s eval_unsafe_arith || true
 
 case $SH in (dash|mksh|zsh) return; esac
 
@@ -562,7 +555,6 @@ vals=
 ## N-I dash/mksh/zsh stdout-json: ""
 
 #### unset assoc errors
-shopt -s eval_unsafe_arith || true
 
 case $SH in (dash|mksh) return; esac
 
@@ -577,7 +569,6 @@ status=0
 
 
 #### Unset array member with dynamic parsing
-shopt -s eval_unsafe_arith
 
 i=1
 a=(w x y z)
@@ -648,7 +639,6 @@ y=
 #### unset a[-1] (bf.bash regression)
 case $SH in (dash|zsh) exit ;; esac
 
-shopt -s eval_unsafe_arith
 a=(1 2 3)
 unset a[-1]
 echo len=${#a[@]}
@@ -678,7 +668,6 @@ last=0
 #### unset a[-1] in sparse array (bf.bash regression)
 case $SH in (dash|zsh) exit ;; esac
 
-shopt -s eval_unsafe_arith
 a=(0 1 2 3 4)
 unset a[1]
 unset a[4]