internal changes to prevent `unset' from scanning associative array subscripts multiple times

CWRU/CWRU.chlog

@@ -2367,3 +2367,36 @@ lib/readline/vi_mode.c
 	  new one, and restore the old one before returning. Prevents some
 	  pointer aliasing problems. From a fuzzing report by
 	  Tillmann Osswald <tosswald@ernw.de>
+
+				   10/29
+				   -----
+arrayfunc.c
+	- tokenize_array_reference: take valid_array_reference and add a third
+	  argument (char **SUBP), which, if non-NULL, gets the null-terminated
+	  subscript parsed from the NAME argument. If it's NULL, the caller
+	  gets the old valid_array_reference behavior. Fix from
+	  Koichi Murase <myoga.murase@gmail.com>
+	- valid_array_reference: just call tokenize_array_reference with a
+	  NULL third argument
+	- unbind_array_element: assume the caller (unset_builtin) passes a
+	  null-terminated SUB that's already been validated by a call to
+	  tokenize_array_reference so we don't need to call skipsubscript() or
+	  check VA_ONEWORD. Fix from Koichi Murase <myoga.murase@gmail.com>
+
+arrayfunc.h
+	- tokenize_array_reference: extern declaration
+
+builtins/set.def
+	- unset_builtin: use tokenize_array_reference to figure out T and pass
+	  that to unbind_array_element. Fix from
+	  Koichi Murase <myoga.murase@gmail.com>
+	- unset_builtin: pass non-null argument to array_variable_part to get
+	  the length of the subscript (T), then cut off any final `]' before
+	  passing it to unbind_array_element, since that's what it now
+	  expects
+
+subst.c
+	- expand_string_for_rhs,expand_string_for_pat: assign td.word from
+	  newly-allocated memory in case it gets freed on error during the
+	  call to call_expand_word_internal(); free it manually when that
+	  call returns

arrayfunc.c

@@ -1064,27 +1064,13 @@ unbind_array_element (var, sub, flags)
      char *sub;
      int flags;
 {
-  int len;
   arrayind_t ind;
   char *akey;
   ARRAY_ELEMENT *ae;
 
-  /* If the caller tells us to treat the entire `sub' as one word, we don't
-     bother to call skipsubscript. */
-  if (var && assoc_p (var) && (flags&VA_ONEWORD))
-    len = strlen (sub) - 1;
-  else
-#if 0
-    len = skipsubscript (sub, 0, (flags&VA_NOEXPAND) || (var && assoc_p(var)));	/* XXX */
-#else
-    len = skipsubscript (sub, 0, (flags&VA_NOEXPAND) | 2);	/* XXX */
-#endif
-  if (sub[len] != ']' || len == 0)
-    {
-      builtin_error ("%s[%s: %s", var->name, sub, _(bash_badsub_errmsg));
-      return -1;
-    }
-  sub[len] = '\0';
+  /* Assume that the caller (unset_builtin) passes us a null-terminated SUB,
+     so we don't have to use VA_ONEWORD or parse the subscript again with
+     skipsubscript(). */
 
   if (ALL_ELEMENT_SUB (sub[0]) && sub[1] == 0)
     {
@@ -1137,7 +1123,7 @@ unbind_array_element (var, sub, flags)
 	    }
 	  /* Fall through for behavior 3 */
 	}
-      ind = array_expand_index (var, sub, len+1, 0);
+      ind = array_expand_index (var, sub, strlen (sub) + 1, 0);
       /* negative subscripts to indexed arrays count back from end */
       if (ind < 0)
 	ind = array_max_index (array_cell (var)) + 1 + ind;
@@ -1153,7 +1139,7 @@ unbind_array_element (var, sub, flags)
   else	/* array_p (var) == 0 && assoc_p (var) == 0 */
     {
       akey = this_command_name;
-      ind = array_expand_index (var, sub, len+1, 0);
+      ind = array_expand_index (var, sub, strlen (sub) + 1, 0);
       this_command_name = akey;
       if (ind == 0)
 	{
@@ -1215,11 +1201,20 @@ print_assoc_assignment (var, quoted)
 
 /* Return 1 if NAME is a properly-formed array reference v[sub]. */
 
+/* Return 1 if NAME is a properly-formed array reference v[sub]. */
+
+/* When NAME is a properly-formed array reference and a non-null argument SUBP
+   is supplied, '[' and ']' that enclose the subscript are replaced by '\0',
+   and the pointer to the subscript in NAME is assigned to *SUBP, so that NAME
+   and SUBP can be later used as the array name and the subscript,
+   respectively.  When SUBP is the null pointer, the original string NAME will
+   not be modified. */
 /* We need to reserve 1 for FLAGS, which we pass to skipsubscript. */
 int
-valid_array_reference (name, flags)
-     const char *name;
+tokenize_array_reference (name, flags, subp)
+     char *name;
      int flags;
+     char **subp;
 {
   char *t;
   int r, len, isassoc;
@@ -1253,16 +1248,34 @@ valid_array_reference (name, flags)
 	 existing associative arrays, using isassoc */
       for (r = 1; r < len; r++)
 	if (whitespace (t[r]) == 0)
-	  return 1;
-      return 0;
-#else
+	  break;
+      if (r == len)
+	return 0; /* Fail if the subscript contains only whitespaces. */
+#endif
+
+      if (subp)
+	{
+	  t[0] = t[len] = '\0';
+	  *subp = t + 1;
+	}
+
       /* This allows blank subscripts */
       return 1;
-#endif
     }
   return 0;
 }
 
+/* Return 1 if NAME is a properly-formed array reference v[sub]. */
+
+/* We need to reserve 1 for FLAGS, which we pass to skipsubscript. */
+int
+valid_array_reference (name, flags)
+     const char *name;
+     int flags;
+{
+  return tokenize_array_reference ((char *)name, flags, (char **)NULL);
+}
+
 /* Expand the array index beginning at S and extending LEN characters. */
 arrayind_t
 array_expand_index (var, s, len, flags)

arrayfunc.h

@@ -1,6 +1,6 @@
 /* arrayfunc.h -- declarations for miscellaneous array functions in arrayfunc.c */
 
-/* Copyright (C) 2001-2020 Free Software Foundation, Inc.
+/* Copyright (C) 2001-2021 Free Software Foundation, Inc.
 
    This file is part of GNU Bash, the Bourne Again SHell.
 
@@ -83,6 +83,8 @@ extern void print_assoc_assignment PARAMS((SHELL_VAR *, int));
 
 extern arrayind_t array_expand_index PARAMS((SHELL_VAR *, char *, int, int));
 extern int valid_array_reference PARAMS((const char *, int));
+extern int tokenize_array_reference PARAMS((char *, int, char **));
+
 extern char *array_value PARAMS((const char *, int, int, int *, arrayind_t *));
 extern char *get_array_value PARAMS((const char *, int, int *, arrayind_t *));
 

builtins/common.c

@@ -992,7 +992,7 @@ builtin_bind_variable (name, value, flags)
   /* Callers are responsible for calling this with array references that have
      already undergone valid_array_reference checks.
      Affected builtins: read, printf
-     To make this really work, needs additional downstream support, starting
+     To make this *really* work, needs additional downstream support, starting
      with assign_array_element and array_variable_name. */
   vflags = assoc_expand_once ? (VA_NOEXPAND|VA_ONEWORD) : 0;
   bindflags = flags | (assoc_expand_once ? ASS_NOEXPAND : 0) | ASS_ALLOWALLSUB;

builtins/set.def

@@ -894,12 +894,8 @@ unset_builtin (list)
 #if defined (ARRAY_VARS)
       unset_array = 0;
       /* XXX valid array reference second arg was 0 */
-      if (!unset_function && nameref == 0 && valid_array_reference (name, vflags))
-	{
-	  t = strchr (name, '[');
-	  *t++ = '\0';
-	  unset_array++;
-	}
+      if (!unset_function && nameref == 0 && tokenize_array_reference (name, vflags, &t))
+	unset_array = 1;
 #endif
       /* Get error checking out of the way first.  The low-level functions
 	 just perform the unset, relying on the caller to verify. */
@@ -987,9 +983,16 @@ unset_builtin (list)
 #if defined (ARRAY_VARS)
 	      if (valid_array_reference (nameref_cell (var), 0))
 		{
+		  int len;
+
 		  tname = savestring (nameref_cell (var));
-		  if (var = array_variable_part (tname, 0, &t, (int *)0))
-		    tem = unbind_array_element (var, t, vflags);	/* XXX new third arg */
+		  if (var = array_variable_part (tname, 0, &t, &len))
+		    {
+		      /* change to what unbind_array_element now expects */
+		      if (t[len - 1] == ']')
+			t[len - 1] = 0;
+		      tem = unbind_array_element (var, t, vflags);	/* XXX new third arg */
+		    }
 		  free (tname);
 		}
 	      else

execute_cmd.c

@@ -1565,6 +1565,7 @@ execute_in_subshell (command, asynchronous, pipe_in, pipe_out, fds_to_close)
   if (running_trap > 0)
     {
       run_trap_cleanup (running_trap - 1);
+itrace("execute_in_subshell: setting running_trap = 0");
       running_trap = 0;		/* XXX - maybe leave this */
     }
 
@@ -4235,7 +4236,8 @@ fix_assignment_words (words)
 
 #if defined (ARRAY_VARS)
 /* Set W_ARRAYREF on words that are valid array references to a builtin that
-   accepts them. This is intended to completely replace assoc_expand_once. */
+   accepts them. This is intended to completely replace assoc_expand_once in
+   time. */
 static void
 fix_arrayref_words (words)
      WORD_LIST *words;

expr.c

@@ -341,7 +341,7 @@ expr_bind_variable (lhs, rhs)
 }
 
 #if defined (ARRAY_VARS)
-/* This is similar to the logic in arrayfunc.c:valid_array_subscript when
+/* This is similar to the logic in arrayfunc.c:valid_array_reference when
    you pass VA_NOEXPAND. */
 static int
 expr_skipsubscript (vp, cp)

subst.c

@@ -4079,9 +4079,10 @@ expand_string_for_rhs (string, quoted, op, pflags, dollar_at_p, expanded_p)
 #else
     td.flags |= W_ASSIGNRHS|W_NOASSNTILDE;		/* expand b in ${a=b} like assignment */
 #endif
-  td.word = string;
+  td.word = savestring (string);
   tresult = call_expand_word_internal (&td, quoted, 1, dollar_at_p, expanded_p);
   expand_no_split_dollar_star = old_nosplit;
+  free (td.word);
 
   return (tresult);
 }
@@ -4103,9 +4104,10 @@ expand_string_for_pat (string, quoted, dollar_at_p, expanded_p)
   oexp = expand_no_split_dollar_star;
   expand_no_split_dollar_star = 1;
   td.flags = W_NOSPLIT2;		/* no splitting, remove "" and '' */
-  td.word = string;
+  td.word = savestring (string);
   tresult = call_expand_word_internal (&td, quoted, 1, dollar_at_p, expanded_p);
   expand_no_split_dollar_star = oexp;
+  free (td.word);
 
   return (tresult);
 }
@@ -10309,7 +10311,7 @@ expand_array_subscript (string, sindex, quoted, flags)
 
   /* string[si] == LBRACK */
   ni = skipsubscript (string, si, 0);
-  /* These checks mirror the ones in valid_array_subscript. The check for
+  /* These checks mirror the ones in valid_array_reference. The check for
      (ni - si) == 1 checks for empty subscripts. We don't check that the
      subscript is a separate word if we're parsing an arithmetic expression. */
   if (ni >= slen || string[ni] != RBRACK || (ni - si) == 1 ||

tests/RUN-ONE-TEST

@@ -1,4 +1,4 @@
-BUILD_DIR=/usr/local/build/chet/bash/bash-current
+BUILD_DIR=/usr/local/build/bash/bash-current
 THIS_SH=$BUILD_DIR/bash
 PATH=$PATH:$BUILD_DIR