From: Chet Ramey Date: Mon, 1 Nov 2021 14:42:50 +0000 (-0400) Subject: internal changes to prevent `unset' from scanning associative array subscripts multip... X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4657c0405034d4126c6d16f4aeb97a3f92f77a26;p=thirdparty%2Fbash.git internal changes to prevent `unset' from scanning associative array subscripts multiple times --- diff --git a/CWRU/CWRU.chlog b/CWRU/CWRU.chlog index 1c564e2ff..18118ccfe 100644 --- a/CWRU/CWRU.chlog +++ b/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 + + 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 + - 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 + +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 + - 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 diff --git a/arrayfunc.c b/arrayfunc.c index 1ca3b365a..00576a9dc 100644 --- a/arrayfunc.c +++ b/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) diff --git a/arrayfunc.h b/arrayfunc.h index 31d91cc80..87569e434 100644 --- a/arrayfunc.h +++ b/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 *)); diff --git a/builtins/common.c b/builtins/common.c index a115ed9ec..fcc248f95 100644 --- a/builtins/common.c +++ b/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; diff --git a/builtins/set.def b/builtins/set.def index 32dad9ab3..5f2f4c153 100644 --- a/builtins/set.def +++ b/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 diff --git a/execute_cmd.c b/execute_cmd.c index 7eaeac8c1..964093c1b 100644 --- a/execute_cmd.c +++ b/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; diff --git a/expr.c b/expr.c index 56c72fd66..73e5b2978 100644 --- a/expr.c +++ b/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) diff --git a/subst.c b/subst.c index c5855f3b2..73c67f888 100644 --- a/subst.c +++ b/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 || diff --git a/tests/RUN-ONE-TEST b/tests/RUN-ONE-TEST index 0b0638107..c8bef8dd1 100755 --- a/tests/RUN-ONE-TEST +++ b/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 diff --git a/tests/exec1.sub b/tests/exec1.sub old mode 100644 new mode 100755 diff --git a/tests/trap1.sub b/tests/trap1.sub old mode 100644 new mode 100755 diff --git a/tests/trap2.sub b/tests/trap2.sub old mode 100644 new mode 100755 diff --git a/tests/trap2a.sub b/tests/trap2a.sub old mode 100644 new mode 100755