From b58c901294c1d5c796e04e41d6bf855a1b593e58 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 19:40:12 -0500 Subject: [PATCH 1/3] fix(indirect): truncate array target at first ] to stop operand recursion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An indirect array target such as r='a[0]:-${!r}]' passed validation whole, so desugar re-parsed ${a[0]:-${!r}]} and the unset-element default re-entered indirect expansion without bound (stack overflow, SIGABRT). Truncate the resolved bracket target at the first ] after [ before validation. Real bash resolves a[0] and ignores the rest (verified oracles), so trailing operands — including nested ${!...} — never reach the re-parse. Plain names, a[$i], a[@], assoc subscripts pass through unchanged; $(/backquote/process-substitution guards unchanged. Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 1 + crates/bashkit/src/interpreter/expansion.rs | 15 ++++ .../tests/integration/threat_model_tests.rs | 74 +++++++++++++++++++ knowledge/security/threat-model.md | 1 + 4 files changed, 91 insertions(+) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 1884be6e6..fe85da51b 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,6 +82,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-134) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets truncate at the first `]` (bash parity: `a[0]` resolves, rest ignored), so trailing operands never reach the indirect re-parse | MITIGATED | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index e95595047..df4c69185 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2624,6 +2624,21 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; + // TM-RES-INDIRECT: an indirect array target ends at the first `]` + // after `[` (real bash resolves `a[0]` from `a[0]:-x]` and ignores + // the rest). Truncating before validation keeps trailing + // operator/nesting text — including a nested `${!...}` — from ever + // reaching the desugar re-parse below, which re-entered indirect + // expansion without bound (`r='a[0]:-${!r}]'` overflowed the stack). + // Targets without a sub-`]` (plain names, `a[$i]`, `a[@]`) pass + // through unchanged. + let value = match value.find('[') { + Some(open) => match value[open..].find(']') { + Some(rel) => value[..open + rel + 1].to_string(), + None => value, + }, + None => value, + }; let valid = ident(&value) || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 6ecbcbc02..35c9a2245 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -20,6 +20,80 @@ use std::time::Duration; mod resource_exhaustion { use super::*; + /// TM-RES-INDIRECT: an indirect array target ends at the first `]`. + /// Trailing operator/nesting text is never re-expanded as code, so a + /// default operand cannot re-enter indirect expansion and exhaust the + /// stack (`r='a[0]:-${!r}]'`). Matches real bash: element-or-empty. + #[tokio::test] + async fn indirect_array_target_ends_at_first_close_bracket() { + // Trailing operator/nesting text is ignored, like real bash + // (verified: bash prints `out=[]`, `st=0` for every target below). + for target in [ + "a[0]:-injected]", + "a[0]-injected]", + "a[0]:=injected]", + "a[0]+injected]", + "a[0]/x/injected]", + "a[0][1]", + "a[0]}${x:-injected}[0]", + ] { + let mut bash = Bash::new(); + let result = bash + .exec(&format!( + "r='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" + )) + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n", "target: {target}"); + assert!( + result.stderr.is_empty(), + "target: {target}: {}", + result.stderr + ); + } + // Set element resolves through the truncated target (bash: `1`). + let mut bash = Bash::new(); + let result = bash + .exec("a=(1 2)\nr='a[0]:-injected]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + + /// TM-RES-INDIRECT: self-referential default operand terminates. + /// Pre-fix this exact reproducer overflowed the stack (SIGABRT). + #[tokio::test] + async fn indirect_self_cycle_default_operand_terminates() { + // Exact finding reproducer: unset element + self-cycle in default. + // Real bash prints `out=[]` with status 0 and never crashes. + let mut bash = Bash::new(); + let result = bash + .exec("unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + // Set element: the cycle operand is discarded, element wins. + let mut bash = Bash::new(); + let result = bash + .exec("a=(1 2)\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + // Mutual cycle through two names terminates as well. + let mut bash = Bash::new(); + let result = bash + .exec( + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + ) + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + /// V1: Test that command limit prevents infinite execution #[tokio::test] async fn threat_infinite_commands_blocked() { diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index bb7631f85..856e8ccc9 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,6 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | +| TM-DOS-134 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow) | The resolved bracket target is truncated at the first `]` after `[` before validation (real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_target_ends_at_first_close_bracket`, `indirect_self_cycle_default_operand_terminates` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101. From 89e45cb22b6d03e97522bde9fa4171956fb70178 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 20:55:20 -0500 Subject: [PATCH 2/3] fix(indirect): preserve nested subscripts with a complete-reference boundary PR 2673 truncated indirect array targets at the first ']', which broke valid nested subscripts (a[b[0]] errored; both real bash versions print two). The boundary now ends at the close of the first complete [...] reference, tracking nested brackets and skipping quoted/escaped text, so trailing operators still never reach the desugar re-parse (self/mutual ${!...} cycles terminate with bash-parity output) while nested and quoted-key references resolve whole. Tests: hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash; assoc direct/indirect agreement. Renumbers the prior TM-DOS-134 rows to TM-DOS-135 (tilde budget work owns 134). Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/expansion.rs | 55 +++++- .../tests/integration/threat_model_tests.rs | 178 ++++++++++++++---- knowledge/security/threat-model.md | 2 +- 4 files changed, 189 insertions(+), 48 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index fe85da51b..b2582a6dd 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,7 +82,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-134) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets truncate at the first `]` (bash parity: `a[0]` resolves, rest ignored), so trailing operands never reach the indirect re-parse | MITIGATED | +| Indirect operand recursion (TM-DOS-135) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets end at the close of the first complete `[...]` reference (bash parity; nested subscripts resolve whole), so trailing operands never reach the indirect re-parse | MITIGATED | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index 42e683879..a91757dba 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2637,6 +2637,40 @@ 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 { + let bytes = value.as_bytes(); + let mut depth = 0usize; + let mut quote: Option = 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 @@ -2673,17 +2707,18 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; - // TM-RES-INDIRECT: an indirect array target ends at the first `]` - // after `[` (real bash resolves `a[0]` from `a[0]:-x]` and ignores - // the rest). Truncating before validation keeps trailing - // operator/nesting text — including a nested `${!...}` — from ever - // reaching the desugar re-parse below, which re-entered indirect - // expansion without bound (`r='a[0]:-${!r}]'` overflowed the stack). - // Targets without a sub-`]` (plain names, `a[$i]`, `a[@]`) pass - // through unchanged. + // TM-RES-INDIRECT: an indirect array target ends at the close of + // the first complete `[...]` reference (real bash resolves `a[0]` + // from `a[0]:-x]` and ignores the rest). The boundary tracks nested + // brackets and quoted text, so valid nested subscripts (`a[b[0]]`, + // `h['a]b']`) survive whole while trailing operator/nesting text — + // including a nested `${!...}` — never reaches the desugar re-parse + // below, which re-entered indirect expansion without bound + // (`r='a[0]:-${!r}]'` overflowed the stack). Targets without a + // balanced `]` (plain names, `a[$i]`, `a[@]`) pass through unchanged. let value = match value.find('[') { - Some(open) => match value[open..].find(']') { - Some(rel) => value[..open + rel + 1].to_string(), + Some(open) => match Self::indirect_array_end(&value, open) { + Some(end) => value[..end].to_string(), None => value, }, None => value, diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 35c9a2245..8b547d3a1 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -10,8 +10,9 @@ use bashkit::{ OverlayFs, SessionLimits, TraceEventDetails, TraceEventKind, TraceMode, }; use std::path::Path; +use std::process::{Command, Stdio}; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; // ============================================================================= // 1. RESOURCE EXHAUSTION TESTS @@ -20,14 +21,93 @@ use std::time::Duration; mod resource_exhaustion { use super::*; - /// TM-RES-INDIRECT: an indirect array target ends at the first `]`. - /// Trailing operator/nesting text is never re-expanded as code, so a - /// default operand cannot re-enter indirect expansion and exhaust the - /// stack (`r='a[0]:-${!r}]'`). Matches real bash: element-or-empty. + /// TM-DOS-135: child-process runner for hostile scripts. + /// Hostile reproducers never run in the test process: pre-fix they + /// overflowed the stack and aborted the whole runner. The CLI binary + /// sits next to the test binary's `deps` directory. + fn bashkit_cli_path() -> std::path::PathBuf { + let exe = std::env::current_exe().expect("current test executable"); + let deps = exe.parent().expect("deps dir"); + let profile = deps.parent().expect("profile dir"); + profile.join(format!("bashkit{}", std::env::consts::EXE_SUFFIX)) + } + + struct BoundedRun { + timed_out: bool, + status_ok: bool, + exit_code: Option, + stdout: String, + stderr: String, + } + + /// Runs a script in a child CLI process under a wall-clock watchdog. + /// Darwin note: Rust std exposes no portable per-child CPU, stack, or + /// memory caps, so the watchdog kill plus the 8 KiB output cap are the + /// enforced bounds here. Records the real producer exit status. + fn run_cli_bounded(script: &str, limit: Duration) -> BoundedRun { + let mut child = Command::new(bashkit_cli_path()) + .args(["-c", script]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn bashkit CLI"); + let start = Instant::now(); + let timed_out = loop { + match child.try_wait().expect("poll child") { + Some(_) => break false, + None => { + if start.elapsed() >= limit { + let _ = child.kill(); + break true; + } + std::thread::sleep(Duration::from_millis(10)); + } + } + }; + let output = child.wait_with_output().expect("collect child output"); + const CAP: usize = 8192; + let mut stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + let mut stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + stdout.truncate(CAP); + stderr.truncate(CAP); + BoundedRun { + timed_out, + status_ok: output.status.success(), + exit_code: output.status.code(), + stdout, + stderr, + } + } + + /// TM-DOS-135: hostile indirect cycles terminate in a bounded child. + /// Exact finding reproducer plus the mutual variant. Real bash prints + /// `out=[]` with status 0 for both and never crashes. + #[test] + fn indirect_hostile_cycles_terminate_in_bounded_child() { + for script in [ + "unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + ] { + let run = run_cli_bounded(script, Duration::from_secs(20)); + assert!(!run.timed_out, "hostile script hung: {script}"); + assert!( + run.status_ok, + "exit={:?} stderr={}", + run.exit_code, run.stderr + ); + assert_eq!(run.stdout, "out=[]\nst=0\n", "script: {script}"); + assert!(run.stderr.is_empty(), "stderr: {}", run.stderr); + } + } + + /// TM-DOS-135: an indirect array target ends at the close of the first + /// complete `[...]` reference; trailing operators are ignored like real + /// bash, while nested subscripts resolve whole. #[tokio::test] - async fn indirect_array_target_ends_at_first_close_bracket() { - // Trailing operator/nesting text is ignored, like real bash - // (verified: bash prints `out=[]`, `st=0` for every target below). + async fn indirect_array_boundary_ignores_trailing_operators() { + // Trailing operator/nesting text resolves the leading element + // (verified against real bash: `out=[1]`, `st=0` for each). for target in [ "a[0]:-injected]", "a[0]-injected]", @@ -40,60 +120,86 @@ mod resource_exhaustion { let mut bash = Bash::new(); let result = bash .exec(&format!( - "r='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" + "a=(1 2)\nr='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" )) .await .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n", "target: {target}"); + assert_eq!(result.stdout, "out=[1]\nst=0\n", "target: {target}"); assert!( result.stderr.is_empty(), "target: {target}: {}", result.stderr ); } - // Set element resolves through the truncated target (bash: `1`). + // Nested subscripts resolve whole (bash: `two`/`two`). let mut bash = Bash::new(); let result = bash - .exec("a=(1 2)\nr='a[0]:-injected]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]'\nprintf \"indirect=[%s]\\n\" \"${!r}\"\nprintf \"direct=[%s]\\n\" \"${a[b[0]]}\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert_eq!(result.stdout, "indirect=[two]\ndirect=[two]\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - } - - /// TM-RES-INDIRECT: self-referential default operand terminates. - /// Pre-fix this exact reproducer overflowed the stack (SIGABRT). - #[tokio::test] - async fn indirect_self_cycle_default_operand_terminates() { - // Exact finding reproducer: unset element + self-cycle in default. - // Real bash prints `out=[]` with status 0 and never crashes. + // An operator after a nested reference is ignored (bash: `two`). let mut bash = Bash::new(); let result = bash - .exec("unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]:-D'\necho \"out=[${!r}]\"\necho \"st=$?\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert_eq!(result.stdout, "out=[two]\nst=0\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // Set element: the cycle operand is discarded, element wins. + // Associative references agree between direct and indirect use. + // Portable note: indexed cases above also run against real bash + // below; assoc arrays need bash 4+, so agreement is asserted here. let mut bash = Bash::new(); let result = bash - .exec("a=(1 2)\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("declare -A h=([k]=V [key]=W)\nkey=k\nr='h[$key]'\necho \"dollar=[${!r}] direct=[${h[$key]}]\"\nr='h[key]'\necho \"plain=[${!r}] direct=[${h[key]}]\"\nr='h[@]'\necho \"at=[${!r}] direct=[${h[@]}]\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n"); - assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // Mutual cycle through two names terminates as well. - let mut bash = Bash::new(); - let result = bash - .exec( - "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", - ) - .await - .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert_eq!( + result.stdout, + "dollar=[V] direct=[V]\nplain=[W] direct=[W]\nat=[V W] direct=[V W]\n" + ); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); } + // Out of scope: `]` inside an assoc key is unsupported by the + // direct subscript parser as well (`declare` accepts it, reads + // misparse), so no indirect agreement is asserted for that shape. + // The boundary scan still tracks quotes so a complete reference + // stays whole if the parser ever learns quoted brackets. + + /// TM-DOS-135: finite nested indexed indirects match real bash + /// byte-for-byte. Indexed arrays work on bash 3.2+, so this runs on + /// macOS and Linux CI alike. + #[tokio::test] + async fn indirect_nested_indexed_matches_real_bash() { + for script in [ + "a=(zero one two three); b=(2); r='a[b[0]]'; printf 'indirect=[%s]\n' \"${!r}\"; printf 'direct=[%s]\n' \"${a[b[0]]}\"", + "a=(zero one two three); b=(2); r='a[b[0]]:-D'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[0][1]'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[1]EXTRA'; echo \"out=[${!r}]\"", + "a=(x y); i=1; r='a[$i]'; echo \"out=[${!r}]\"", + ] { + let expected = Command::new("bash") + .args(["--noprofile", "--norc", "-c", script]) + .output() + .expect("run real bash"); + assert!( + expected.status.success(), + "real bash failed: {script}: {}", + String::from_utf8_lossy(&expected.stderr) + ); + let mut harness = Bash::new(); + let result = harness.exec(script).await.unwrap(); + assert_eq!( + result.stdout, + String::from_utf8_lossy(&expected.stdout).into_owned(), + "script: {script}" + ); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + } + /// V1: Test that command limit prevents infinite execution #[tokio::test] async fn threat_infinite_commands_blocked() { diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 856e8ccc9..bf09fc77b 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,7 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | -| TM-DOS-134 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow) | The resolved bracket target is truncated at the first `]` after `[` before validation (real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_target_ends_at_first_close_bracket`, `indirect_self_cycle_default_operand_terminates` | **MITIGATED** | +| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | The resolved bracket target ends at the close of the first complete `[...]` reference (nesting- and quote-aware; real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_ignores_trailing_operators`, `indirect_hostile_cycles_terminate_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101. From 164aed26e0cb16fb089e70796b5fc7ab3b2e48c5 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 21:21:34 -0500 Subject: [PATCH 3/3] fix(indirect): refuse trailing text unless the array reference is complete Modern bash (5.x) rejects indirect array targets with trailing operator/nesting text (invalid variable name); older 3.2 ignored the tail. Refusal (instead of stripping) keeps nested ${!...} from the desugar re-parse while valid nested subscripts (a[b[0]]) resolve whole, matching bash 5.x in every probed shape. Hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash on both bash generations for complete references; assoc direct/indirect agreement. TM-DOS-135 (tilde owns 134). Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/expansion.rs | 54 ++++++------- .../tests/integration/threat_model_tests.rs | 76 ++++++++++--------- knowledge/security/threat-model.md | 2 +- 4 files changed, 71 insertions(+), 63 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index b2582a6dd..c7a17f912 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,7 +82,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 | Bracket targets end at the close of the first complete `[...]` reference (bash parity; nested subscripts resolve whole), so trailing operands never reach the indirect re-parse | 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 | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index a91757dba..bb01309f4 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2707,34 +2707,36 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; - // TM-RES-INDIRECT: an indirect array target ends at the close of - // the first complete `[...]` reference (real bash resolves `a[0]` - // from `a[0]:-x]` and ignores the rest). The boundary tracks nested - // brackets and quoted text, so valid nested subscripts (`a[b[0]]`, - // `h['a]b']`) survive whole while trailing operator/nesting text — - // including a nested `${!...}` — never reaches the desugar re-parse + // TM-RES-INDIRECT: a bracket indirect target is only valid when + // the first complete `[...]` reference consumes the entire value. + // Modern bash (5.x) rejects trailing operator/nesting text with + // `invalid variable name` (older 3.2 ignored it); rejection also + // keeps a nested `${!...}` from ever reaching the desugar re-parse // below, which re-entered indirect expansion without bound - // (`r='a[0]:-${!r}]'` overflowed the stack). Targets without a - // balanced `]` (plain names, `a[$i]`, `a[@]`) pass through unchanged. - let value = match value.find('[') { - Some(open) => match Self::indirect_array_end(&value, open) { - Some(end) => value[..end].to_string(), - None => value, - }, - None => value, + // (`r='a[0]:-${!r}]'` overflowed the stack). The boundary tracks + // nested brackets and quoted text, so valid nested subscripts + // (`a[b[0]]`) resolve whole, while `a[0]:-x]`, `a[0][1]`, and + // 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()) + } + None => true, }; - let valid = ident(&value) - || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) - || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") - || value.find('[').is_some_and(|open| { - ident(&value[..open]) - && value.ends_with(']') - && value.len() > open + 2 - && !value.contains("$(") - && !value.contains('`') - && !value.contains("<(") - && !value.contains(">(") - }); + let valid = valid_boundary + && (ident(&value) + || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) + || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") + || value.find('[').is_some_and(|open| { + ident(&value[..open]) + && value.ends_with(']') + && value.len() > open + 2 + && !value.contains("$(") + && !value.contains('`') + && !value.contains("<(") + && !value.contains(">(") + })); if !valid { // THREAT[TM-DOS-130]: echo a bounded prefix of the bad target. let shown: String = value.chars().take(256).collect(); diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 8b547d3a1..9cf3d67f7 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -34,7 +34,6 @@ mod resource_exhaustion { struct BoundedRun { timed_out: bool, - status_ok: bool, exit_code: Option, stdout: String, stderr: String, @@ -73,41 +72,54 @@ mod resource_exhaustion { stderr.truncate(CAP); BoundedRun { timed_out, - status_ok: output.status.success(), exit_code: output.status.code(), stdout, stderr, } } - /// TM-DOS-135: hostile indirect cycles terminate in a bounded child. - /// Exact finding reproducer plus the mutual variant. Real bash prints - /// `out=[]` with status 0 for both and never crashes. + /// TM-DOS-135: hostile indirect cycles are refused in a bounded child. + /// Exact finding reproducer plus the mutual variant. Both run in a + /// child CLI under a wall-clock watchdog (never in the test runner: + /// pre-fix they overflowed the stack and aborted the process) and are + /// refused with `invalid variable name`, like modern bash (5.x); the + /// real producer exit status is asserted, never relabeled. #[test] - fn indirect_hostile_cycles_terminate_in_bounded_child() { + fn indirect_hostile_cycles_rejected_in_bounded_child() { for script in [ - "unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", - "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + "unset a\nr='a[0]:-${!r}]'\necho \"${!r}\"", + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"${!r}\"", ] { let run = run_cli_bounded(script, Duration::from_secs(20)); assert!(!run.timed_out, "hostile script hung: {script}"); + assert_eq!( + run.exit_code, + Some(1), + "script: {script}, stdout: {}, stderr: {}", + run.stdout, + run.stderr + ); + assert!(run.stdout.is_empty(), "stdout: {}", run.stdout); assert!( - run.status_ok, - "exit={:?} stderr={}", - run.exit_code, run.stderr + run.stderr.contains("invalid variable name"), + "stderr: {}", + run.stderr ); - assert_eq!(run.stdout, "out=[]\nst=0\n", "script: {script}"); - assert!(run.stderr.is_empty(), "stderr: {}", run.stderr); + assert!(run.stderr.len() < 1024, "stderr: {}", run.stderr); } } - /// TM-DOS-135: an indirect array target ends at the close of the first - /// complete `[...]` reference; trailing operators are ignored like real - /// bash, while nested subscripts resolve whole. + /// TM-DOS-135: a bracket indirect target is refused unless the first + /// complete `[...]` reference consumes the whole value. Modern bash + /// (5.x) rejects trailing operators with `invalid variable name`; + /// refusal also keeps a nested `${!...}` from ever reaching the desugar + /// re-parse, which re-entered indirect expansion without bound. + /// Valid nested subscripts resolve whole. #[tokio::test] - async fn indirect_array_boundary_ignores_trailing_operators() { - // Trailing operator/nesting text resolves the leading element - // (verified against real bash: `out=[1]`, `st=0` for each). + async fn indirect_array_boundary_rejects_trailing_operators() { + // Trailing operator/nesting text is refused, like bash 5.x. Older + // bash 3.2 ignored the tail, so no real-bash oracle is asserted + // for these shapes; the rejection itself is the contract. for target in [ "a[0]:-injected]", "a[0]-injected]", @@ -116,21 +128,24 @@ mod resource_exhaustion { "a[0]/x/injected]", "a[0][1]", "a[0]}${x:-injected}[0]", + "a[b[0]]:-D", ] { let mut bash = Bash::new(); let result = bash - .exec(&format!( - "a=(1 2)\nr='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" - )) + .exec(&format!("a=(1 2)\nb=(0)\nr='{target}'\necho \"${{!r}}\"")) .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n", "target: {target}"); + assert_ne!(result.exit_code, 0, "target accepted: {target}"); + assert_eq!(result.exit_code, 1, "target: {target}"); + assert!(result.stdout.is_empty(), "target: {target}"); assert!( - result.stderr.is_empty(), + result.stderr.contains("invalid variable name"), "target: {target}: {}", result.stderr ); + assert!(result.stderr.len() < 1024, "target: {target}"); } + // Nested subscripts resolve whole (bash: `two`/`two`). let mut bash = Bash::new(); let result = bash @@ -139,14 +154,6 @@ mod resource_exhaustion { .unwrap(); assert_eq!(result.stdout, "indirect=[two]\ndirect=[two]\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // An operator after a nested reference is ignored (bash: `two`). - let mut bash = Bash::new(); - let result = bash - .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]:-D'\necho \"out=[${!r}]\"\necho \"st=$?\"") - .await - .unwrap(); - assert_eq!(result.stdout, "out=[two]\nst=0\n"); - assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); // Associative references agree between direct and indirect use. // Portable note: indexed cases above also run against real bash // below; assoc arrays need bash 4+, so agreement is asserted here. @@ -175,9 +182,8 @@ mod resource_exhaustion { async fn indirect_nested_indexed_matches_real_bash() { for script in [ "a=(zero one two three); b=(2); r='a[b[0]]'; printf 'indirect=[%s]\n' \"${!r}\"; printf 'direct=[%s]\n' \"${a[b[0]]}\"", - "a=(zero one two three); b=(2); r='a[b[0]]:-D'; echo \"out=[${!r}]\"", - "a=(1 2); r='a[0][1]'; echo \"out=[${!r}]\"", - "a=(1 2); r='a[1]EXTRA'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[0]'; echo \"out=[${!r}]\"", + "a=(x y); r='a[@]'; echo \"out=[${!r}]\"", "a=(x y); i=1; r='a[$i]'; echo \"out=[${!r}]\"", ] { let expected = Command::new("bash") diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index bf09fc77b..035d9bcaf 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,7 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | -| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | The resolved bracket target ends at the close of the first complete `[...]` reference (nesting- and quote-aware; real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_ignores_trailing_operators`, `indirect_hostile_cycles_terminate_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | +| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | A bracket target is valid only when the first complete `[...]` reference (nesting- and quote-aware) consumes the whole value; any trailing text is refused with `invalid variable name`, like modern bash 5.x (older 3.2 ignored the tail). Refusal keeps a nested `${!...}` from ever reaching the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_rejects_trailing_operators`, `indirect_hostile_cycles_rejected_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101.