fix(parser): unify indirect subscript consumption

crates/bashkit/docs/threat-model.md

@@ -83,7 +83,7 @@ through configurable limits.
 | History files (TM-DOS-131) | `history -a` loops, huge `$HISTSIZE`, giant `$HISTFILE` | History list bounded by `max_history_entries`/`max_history_bytes`; `$HISTFILE` reads refused past `max_input_bytes`; VFS only | MITIGATED |
 | Completion specs and bindings (TM-DOS-132) | `complete`/`bind` in a loop | 1024 specs (64 KiB each) and 1024 binding changes (4 KiB each) per interpreter; never shared between instances | MITIGATED |
 | Prompt recursion (TM-DOS-133) | `x='${x@P}'; echo "${x@P}"`, mutual cycles or substitution-mediated cycles | Cumulative prompt depth capped at `min(max_ast_depth, 8)`; shared work, aggregate input, cancellation and deadline checks on each prompt; descendant shells inherit depth | MITIGATED |
-| Indirect operand recursion (TM-DOS-135) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | A bracket target is refused unless the first complete `[...]` reference consumes the whole value (bash 5.x parity; nested subscripts resolve whole) | MITIGATED |
+| Indirect operand recursion (TM-DOS-135) | Indirect target text reinterpreted as an operator/default operand | Admission and parameter parsing share an exact-reference scanner for nested brackets, braces, quotes, and escapes; indirect reparses inherit request work/cancellation and enclosing substitution depth | MITIGATED |
 
 **Stack Overflow / Recursion:**
 

crates/bashkit/src/interpreter/expansion.rs

@@ -2749,40 +2749,6 @@ impl Interpreter {
         })
     }
 
-    // End index (exclusive) of the first complete subscript reference that
-    // opens at `open`: nested brackets nest, quoted or escaped text is
-    // skipped. `None` when the brackets never balance. All boundary bytes
-    // are ASCII, so the index is always a char boundary.
-    fn indirect_array_end(value: &str, open: usize) -> Option<usize> {
-        let bytes = value.as_bytes();
-        let mut depth = 0usize;
-        let mut quote: Option<u8> = None;
-        let mut escaped = false;
-        let mut i = open;
-        while i < bytes.len() {
-            let b = bytes[i];
-            // 0x27 is a single quote, 0x22 a double quote, 0x5c a backslash.
-            if escaped {
-                escaped = false;
-            } else if quote.is_some_and(|q| b == q) {
-                quote = None;
-            } else if quote.is_none() && (b == 0x27 || b == 0x22) {
-                quote = Some(b);
-            } else if quote.is_none() && b == 0x5c {
-                escaped = true;
-            } else if quote.is_none() && b == b'[' {
-                depth += 1;
-            } else if quote.is_none() && b == b']' && depth > 0 {
-                depth -= 1;
-                if depth == 0 {
-                    return Some(i + 1);
-                }
-            }
-            i += 1;
-        }
-        None
-    }
-
     /// The parameter `${!name...}` refers to: the value of `name` (an
     /// element when it carries a subscript). bash rejects an unset `name`
     /// ("invalid indirect expansion") and a value that is not a parameter
@@ -2831,9 +2797,8 @@ impl Interpreter {
         // unbalanced shapes are refused. Plain names, `a[$i]`, and `a[@]`
         // have no trailing text and pass through unchanged.
         let valid_boundary = match value.find('[') {
-            Some(open) => {
-                Self::indirect_array_end(&value, open).is_some_and(|end| end == value.len())
-            }
+            Some(open) => crate::parser::parameter_subscript_end(&value, open)
+                .is_some_and(|end| end == value.len()),
             None => true,
         };
         let valid = valid_boundary
@@ -2914,11 +2879,13 @@ impl Interpreter {
                 suffix
             };
             let text = format!("${{{target}{suffix}}}");
-            let parsed = Parser::parse_word_string_with_limits(
+            let parsed = Parser::parse_indirect_word(
                 &text,
                 self.limits.max_ast_depth,
                 self.limits.max_parser_operations,
-            );
+                self.counters.subst_depth,
+                self.execution_budget.clone(),
+            )?;
             let [new_part] = <[WordPart; 1]>::try_from(parsed.parts).map_err(|_| {
                 crate::error::Error::LineAbort(self.diag(format!("{text}: bad substitution\n")))
             })?;

crates/bashkit/src/parser/mod.rs

@@ -290,6 +290,26 @@ impl<'a> Parser<'a> {
         parser.parse_word(input.to_string())
     }
 
+    /// Indirect reparses share request work/cancellation and enclosing depth.
+    pub(crate) fn parse_indirect_word(
+        input: &str,
+        max_depth: usize,
+        max_fuel: usize,
+        current_depth: usize,
+        budget: crate::limits::ExecutionBudget,
+    ) -> Result<Word> {
+        budget.consume_work(input.len() as u64)?;
+        let mut parser =
+            Parser::with_limits(input, max_depth, max_fuel).with_execution_budget(budget.clone());
+        parser.current_depth = current_depth;
+        let word = parser.parse_word(input.to_string());
+        budget.check()?;
+        if let Some(error) = parser.deferred_error.take() {
+            return Err(error);
+        }
+        Ok(word)
+    }
+
     /// Parse `input` as an unquoted here-document body: expansions apply,
     /// `"` is literal, and only `\$`, `` \` `` and `\\` are escapes. Used for
     /// prompt strings (`${x@P}`, PS1), which bash expands the same way.
@@ -4270,35 +4290,7 @@ impl<'a> Parser<'a> {
             if chars.peek() == Some(&'[') {
                 chars.next(); // consume '['
                 let mut index = String::new();
-                // Track nesting so nested ${...} containing
-                // brackets (e.g. ${#arr[@]}) don't prematurely
-                // close the subscript.
-                let mut bracket_depth: i32 = 0;
-                let mut brace_depth: i32 = 0;
-                while let Some(&c) = chars.peek() {
-                    if c == ']' && bracket_depth == 0 && brace_depth == 0 {
-                        chars.next();
-                        break;
-                    }
-                    match c {
-                        '[' => bracket_depth += 1,
-                        ']' => bracket_depth -= 1,
-                        '$' => {
-                            index.push(chars.next().unwrap());
-                            if chars.peek() == Some(&'{') {
-                                brace_depth += 1;
-                                index.push(chars.next().unwrap());
-                                continue;
-                            }
-                            continue;
-                        }
-                        '{' => brace_depth += 1,
-                        '}' if brace_depth > 0 => brace_depth -= 1,
-                        '}' => {}
-                        _ => {}
-                    }
-                    index.push(chars.next().unwrap());
-                }
+                scan_parameter_subscript(chars, |c| index.push(c));
                 // Strip surrounding quotes from index (e.g. "foo" -> foo)
                 if index.len() >= 2
                     && ((index.starts_with('"') && index.ends_with('"'))
@@ -5135,6 +5127,47 @@ fn empty_background(span: Span) -> (ListOperator, Command) {
     )
 }
 
+/// Consume a subscript after `[`, preserving its source text in `emit`.
+/// THREAT[TM-DOS-135]: admission and parameter parsing must agree on its end.
+fn scan_parameter_subscript(
+    chars: &mut impl Iterator<Item = char>,
+    mut emit: impl FnMut(char),
+) -> Option<usize> {
+    let mut brackets = 0usize;
+    let mut braces = 0usize;
+    let mut quote = None;
+    let mut escaped = false;
+    let mut consumed = 0usize;
+    for c in chars.by_ref() {
+        consumed += c.len_utf8();
+        if escaped {
+            escaped = false;
+        } else {
+            match (quote, c) {
+                (Some('\''), '\'') => quote = None,
+                (Some('\''), _) => {}
+                (_, '\\' | '\x00') => escaped = true,
+                (Some('"'), '"') => quote = None,
+                (Some(_), _) => {}
+                (None, '\'' | '"') => quote = Some(c),
+                (None, '{') => braces += 1,
+                (None, '}') => braces = braces.saturating_sub(1),
+                (None, '[') => brackets += 1,
+                (None, ']') if brackets == 0 && braces == 0 => return Some(consumed),
+                (None, ']') => brackets = brackets.saturating_sub(1),
+                _ => {}
+            }
+        }
+        emit(c);
+    }
+    None
+}
+
+pub(crate) fn parameter_subscript_end(value: &str, open: usize) -> Option<usize> {
+    let rest = value.get(open..)?.strip_prefix('[')?;
+    scan_parameter_subscript(&mut rest.chars(), |_| {}).map(|used| open + 1 + used)
+}
+
 /// Whether a raw `=~` operand holds an expansion. A `$` that cannot start
 /// one (`^a$`, `(x$|y$)`) is the regex end anchor; single quotes hide both.
 /// Text inside `${...}` when bash would reject it with "bad substitution"
@@ -5171,26 +5204,8 @@ fn bad_brace_parameter(chars: &std::iter::Peekable<std::str::Chars<'_>>) -> Opti
                 it.next();
             }
             if it.peek() == Some(&'[') {
-                // Subscript: balanced brackets; unterminated ones are bad.
-                let mut depth = 0usize;
-                let mut closed = false;
-                for c in it.by_ref() {
-                    match c {
-                        _ if MARKERS.contains(&c) => return None,
-                        '[' => depth += 1,
-                        ']' => {
-                            depth -= 1;
-                            if depth == 0 {
-                                closed = true;
-                                break;
-                            }
-                        }
-                        _ => {}
-                    }
-                }
-                if !closed {
-                    return None;
-                }
+                it.next();
+                scan_parameter_subscript(&mut it, |_| {})?;
             }
             next_is_bad(it.peek().copied(), length_prefix)
         }
@@ -5727,6 +5742,116 @@ mod tests {
     use super::*;
     use std::time::Duration;
 
+    #[test]
+    fn parameter_subscript_consumption_preserves_escaped_and_quoted_brackets() {
+        for (source, expected) in [
+            (r"${items[left\]right]}", r"left\]right"),
+            (r#"${items["left]right"]}"#, "left]right"),
+            (r"${items['left]right']}", "left]right"),
+            (r"${items[nested[0]]}", "nested[0]"),
+            (r"${items[${key}]}", "${key}"),
+        ] {
+            let parsed = Parser::parse_word_string(source);
+            assert!(
+                matches!(
+                    parsed.parts.as_slice(),
+                    [WordPart::ArrayAccess { name, index }]
+                        if name == "items" && index == expected
+                ),
+                "unexpected parts for {source}: {:?}",
+                parsed.parts
+            );
+        }
+    }
+
+    #[test]
+    fn parameter_subscript_consumption_keeps_explicit_operator_after_reference() {
+        let parsed = Parser::parse_word_string(r"${items[left\]right]:-fallback}");
+        assert!(matches!(
+            parsed.parts.as_slice(),
+            [WordPart::ParameterExpansion {
+                name,
+                operator: ParameterOp::UseDefault,
+                operand,
+                colon_variant: true,
+            }] if name == r"items[left\]right]" && operand == "fallback"
+        ));
+    }
+
+    #[test]
+    fn parameter_subscript_consumption_admits_exact_reference_only() {
+        for value in [
+            r"items[left\]right]",
+            r#"items["left]right"]"#,
+            r#"items["left\"]right"]"#,
+            "items[nested[0]]",
+            "items[${key}]",
+            "items[caf\u{e9}]",
+        ] {
+            assert_eq!(
+                parameter_subscript_end(value, 5),
+                Some(value.len()),
+                "{value}"
+            );
+        }
+        for value in ["items[0]:-fallback", "items[0][1]"] {
+            assert_ne!(
+                parameter_subscript_end(value, 5),
+                Some(value.len()),
+                "{value}"
+            );
+        }
+        for value in [r"items[left\]", "items['left]", "items[${key]"] {
+            assert_eq!(parameter_subscript_end(value, 5), None, "{value}");
+        }
+    }
+
+    #[test]
+    fn indirect_reparse_admission_inherits_depth_and_request_failures() {
+        use crate::limits::{ExecutionBudget, ExecutionLimits};
+        use std::sync::atomic::AtomicBool;
+
+        let budget = || {
+            ExecutionBudget::new(
+                &ExecutionLimits::default(),
+                Arc::new(AtomicBool::new(false)),
+            )
+        };
+        let input = "$(printf ok)";
+        assert!(Parser::parse_indirect_word(input, 1, 100, 0, budget()).is_ok());
+        let error = Parser::parse_indirect_word(input, 1, 100, 1, budget()).unwrap_err();
+        assert!(
+            error.to_string().contains("AST nesting too deep"),
+            "{error}"
+        );
+
+        let empty_work = ExecutionBudget::new(
+            &ExecutionLimits::default().max_work_units(0),
+            Arc::new(AtomicBool::new(false)),
+        );
+        assert!(matches!(
+            Parser::parse_indirect_word("${items[0]}", 1, 100, 0, empty_work),
+            Err(Error::ResourceLimit(_))
+        ));
+        let cancelled =
+            ExecutionBudget::new(&ExecutionLimits::default(), Arc::new(AtomicBool::new(true)));
+        assert!(matches!(
+            Parser::parse_indirect_word("${items[0]}", 1, 100, 0, cancelled),
+            Err(Error::Cancelled)
+        ));
+
+        let text = "${items[0]}";
+        let shared = ExecutionBudget::new(
+            &ExecutionLimits::default().max_work_units(text.len() as u64),
+            Arc::new(AtomicBool::new(false)),
+        );
+        assert!(Parser::parse_indirect_word(text, 1, 100, 0, shared.clone()).is_ok());
+        assert!(matches!(
+            Parser::parse_indirect_word(text, 1, 100, 0, shared),
+            Err(Error::ResourceLimit(_))
+        ));
+    }
+
     #[test]
     fn test_parse_simple_command() {
         let parser = Parser::new("echo hello");