diff --git a/crates/bashkit/src/interpreter/arithmetic.rs b/crates/bashkit/src/interpreter/arithmetic.rs index 649d8129d..2c8fdfdec 100644 --- a/crates/bashkit/src/interpreter/arithmetic.rs +++ b/crates/bashkit/src/interpreter/arithmetic.rs @@ -17,6 +17,9 @@ //! (`MAX_ARITHMETIC_EXPANSION_FUEL`) are shared by textual dollar expansion, //! subscript evaluation, and recursive variable evaluation. Check before //! expansion; read-only subscript evaluators borrow the same budget. +//! - Expanded expression/key buffers enforce the byte cap and reserve shared +//! live-memory capacity before growth. Each append charges work and checks +//! cancellation/deadline, including borrowed scalar substitutions. use super::*; @@ -987,7 +990,7 @@ impl Interpreter { let mut budget = ArithmeticBudget::new(); let mut ev = ArithEval::new(self, &mut budget); match ev.expand_name_or_array_element(name) { - Ok(value) => value, + Ok(value) => value.into_inner(), Err(msg) => { self.record_arith_error(msg); String::new() @@ -997,67 +1000,157 @@ impl Interpreter { } impl ArithEval<'_, '_> { - fn expand_variable(&mut self, name: &str) -> ArithResult { + fn new_text(&self) -> ArithResult { + BudgetedString::new(Some(&self.interp.execution_budget)).map_err(|e| e.to_string()) + } + + // THREAT[TM-DOS-026]: admit expanded bytes before allocation, even when + // the final array writer would reject them. The shared checkpoint is + // synchronous so the async timeout need not preempt this scanner. + fn append_text(interp: &Interpreter, out: &mut BudgetedString, text: &str) -> ArithResult<()> { + if out.len().saturating_add(text.len()) > Interpreter::MAX_ARITHMETIC_EXPANSION_BYTES { + return Err("expression too long".to_string()); + } + interp + .execution_budget + .consume_work(text.len().max(1) as u64) + .map_err(|e| e.to_string())?; + out.try_push_str(text).map_err(|e| e.to_string()) + } + + fn copy_text(&self, text: &str) -> ArithResult { + let mut out = self.new_text()?; + self.interp + .execution_budget + .consume_work(text.len().max(1) as u64) + .map_err(|e| e.to_string())?; + out.try_push_str(text).map_err(|e| e.to_string())?; + Ok(out) + } + + fn join_array(&self, name: &str, separator: &str) -> ArithResult { + let name = self.interp.resolve_nameref(name); + let mut out = self.new_text()?; + let mut first = true; + let mut append = |value: &str| { + if !first { + Self::append_text(self.interp, &mut out, separator)?; + } + first = false; + Self::append_text(self.interp, &mut out, value) + }; + if let Some(arr) = self.interp.scoped.assoc_arrays.get(name) { + // Admit the ordering vector and stable sort's scratch storage. + let _lease = self + .interp + .execution_budget + .lease_bytes( + arr.len() + .saturating_mul(std::mem::size_of::<&String>()) + .saturating_mul(2), + ) + .map_err(|e| e.to_string())?; + for key in super::declare::assoc_keys_bash_order(arr) { + append(&arr[key])?; + } + } else if let Some(arr) = self.interp.scoped.arrays.get(name) { + let _lease = self + .interp + .execution_budget + .lease_bytes( + arr.len() + .saturating_mul(std::mem::size_of::<(&usize, &String)>()), + ) + .map_err(|e| e.to_string())?; + let mut items: Vec<_> = arr.iter().collect(); + items.sort_unstable_by_key(|(i, _)| **i); + for (_, value) in items { + append(value)?; + } + } else if let Some(value) = self.interp.lookup_regular_variable_ref(name) { + append(value)?; + } + Ok(out) + } + + fn expand_variable(&mut self, name: &str) -> ArithResult { self.enter()?; let result = { let resolved = self.interp.resolve_nameref(name); if parse_embedded_array_ref(resolved).is_some() { self.expand_name_or_array_element(resolved) } else { - Ok(self.interp.expand_variable(resolved)) + self.copy_text(&self.interp.expand_variable_cow(resolved)) } }; self.leave(); result } - fn expand_variable_or_literal(&mut self, sub: &str) -> ArithResult { + fn expand_variable_or_literal(&mut self, sub: &str) -> ArithResult { + self.budget.charge(sub)?; + self.interp + .execution_budget + .consume_work(sub.len().max(1) as u64) + .map_err(|e| e.to_string())?; let trimmed = sub.trim(); if !trimmed.contains(['"', '\'', '\\']) && trimmed.matches('$').count() <= 1 && let Some(name) = trimmed.strip_prefix('$') { - return self.expand_variable(name.trim_start_matches('{').trim_end_matches('}')); + let key = self.expand_variable(name.trim_start_matches('{').trim_end_matches('}'))?; + if key.len() > Interpreter::MAX_ARITHMETIC_EXPANSION_BYTES { + return Err("expression too long".to_string()); + } + return Ok(key); } if !sub.contains(['$', '"', '\'', '\\']) { - return Ok(sub.to_string()); + return self.copy_text(sub); } - Interpreter::expand_key_text_using(sub, |name, braced| { - if braced { - if let Some(array) = name - .strip_suffix("[@]") - .or_else(|| name.strip_suffix("[*]")) - { - let separator = if name.ends_with("[*]") { - self.interp.get_ifs_separator() - } else { - " ".to_string() - }; - return Ok(self - .interp - .array_values(self.interp.resolve_nameref(array)) - .join(&separator)); + let mut result = self.new_text()?; + let interp = self.interp; + Interpreter::scan_key_text( + sub, + |name, braced| { + if braced { + if let Some(array) = name + .strip_suffix("[@]") + .or_else(|| name.strip_suffix("[*]")) + { + let separator = if name.ends_with("[*]") { + self.interp.get_ifs_separator() + } else { + " ".to_string() + }; + return self.join_array(array, &separator); + } + self.expand_brace_expr_in_arithmetic(name) + } else { + self.expand_variable(name) } - self.expand_brace_expr_in_arithmetic(name) - } else { - self.expand_variable(name) - } - }) + }, + |text| Self::append_text(interp, &mut result, text), + )?; + Ok(result) } /// Textual `$` expansion inside an arithmetic expression (`$x`, `${..}`, /// `$1`, `$#`, ...). Bare identifiers are left for the evaluator. /// Double quotes are dropped (`"$x"` is `$x`). - fn expand_arith_dollars(&mut self, expr: &str) -> ArithResult { + fn expand_arith_dollars(&mut self, expr: &str) -> ArithResult { if !expr.contains('$') && !expr.contains('"') { - return Ok(expr.to_string()); + return self.copy_text(expr); + } + let mut unquoted = self.new_text()?; + for ch in expr.chars().filter(|ch| *ch != '"') { + Self::append_text(self.interp, &mut unquoted, ch.encode_utf8(&mut [0; 4]))?; } - let expr = expr.replace('"', ""); - let mut result = String::new(); + let expr = unquoted; + let mut result = self.new_text()?; let mut chars = expr.chars().peekable(); while let Some(ch) = chars.next() { if ch != '$' { - result.push(ch); + Self::append_text(self.interp, &mut result, ch.encode_utf8(&mut [0; 4]))?; continue; } if chars.peek() == Some(&'(') && chars.clone().nth(1) == Some('(') { @@ -1065,7 +1158,7 @@ impl ArithEval<'_, '_> { // `$((1 + $((2)) ))`): its value, evaluated here. chars.next(); chars.next(); - let mut inner = String::new(); + let mut inner = self.new_text()?; let mut depth = 0i32; while let Some(c) = chars.next() { match c { @@ -1077,7 +1170,7 @@ impl ArithEval<'_, '_> { ')' => depth -= 1, _ => {} } - inner.push(c); + Self::append_text(self.interp, &mut inner, c.encode_utf8(&mut [0; 4]))?; } // THREAT[TM-DOS-130]: nesting recurses; bash-like scripts need few levels. if inner.matches("$((").count() > 32 { @@ -1085,11 +1178,11 @@ impl ArithEval<'_, '_> { } else { let value = ArithEval::new(self.interp, self.budget).eval_with_dollars(&inner)?; - result.push_str(&value.to_string()); + Self::append_text(self.interp, &mut result, &value.to_string())?; } } else if chars.peek() == Some(&'{') { chars.next(); - let mut brace_content = String::new(); + let mut brace_content = self.new_text()?; let mut brace_depth = 1i32; for c in chars.by_ref() { if c == '{' { @@ -1100,28 +1193,36 @@ impl ArithEval<'_, '_> { break; } } - brace_content.push(c); + Self::append_text(self.interp, &mut brace_content, c.encode_utf8(&mut [0; 4]))?; } - result.push_str(&self.expand_brace_expr_in_arithmetic(&brace_content)?); + Self::append_text( + self.interp, + &mut result, + &self.expand_brace_expr_in_arithmetic(&brace_content)?, + )?; } else if let Some(&c) = chars.peek() && matches!(c, '#' | '?' | '$' | '!' | '@' | '*' | '-' | '0'..='9') { chars.next(); - result.push_str(&self.expand_variable(&c.to_string())?); + Self::append_text( + self.interp, + &mut result, + &self.expand_variable(&c.to_string())?, + )?; } else { - let mut name = String::new(); + let mut name = self.new_text()?; while let Some(&c) = chars.peek() { if c.is_ascii_alphanumeric() || c == '_' { - name.push(c); + Self::append_text(self.interp, &mut name, c.encode_utf8(&mut [0; 4]))?; chars.next(); } else { break; } } if name.is_empty() { - result.push('$'); + Self::append_text(self.interp, &mut result, "$")?; } else { - result.push_str(&self.expand_variable(&name)?); + Self::append_text(self.interp, &mut result, &self.expand_variable(&name)?)?; } } } @@ -1130,33 +1231,33 @@ impl ArithEval<'_, '_> { /// Expand a `${...}` expression encountered inside arithmetic context. /// Handles: `${#arr[@]}`, `${#arr[*]}`, `${#var}`, `${arr[idx]}`, `${var}`. - fn expand_brace_expr_in_arithmetic(&mut self, inner: &str) -> ArithResult { + fn expand_brace_expr_in_arithmetic(&mut self, inner: &str) -> ArithResult { // ${#arr[@]} or ${#arr[*]} — array length if let Some(rest) = inner.strip_prefix('#') { if let Some(bracket) = rest.find('[') { if !rest.ends_with(']') { - return Ok("0".to_string()); + return self.copy_text("0"); } let end = rest.len() - 1; if bracket + 1 > end { - return Ok("0".to_string()); + return self.copy_text("0"); } let arr_name = &rest[..bracket]; let idx = &rest[bracket + 1..end]; if idx == "@" || idx == "*" { if let Some(arr) = self.interp.scoped.arrays.get(arr_name) { - return Ok(arr.len().to_string()); + return self.copy_text(&arr.len().to_string()); } if let Some(arr) = self.interp.scoped.assoc_arrays.get(arr_name) { - return Ok(arr.len().to_string()); + return self.copy_text(&arr.len().to_string()); } - return Ok("0".to_string()); + return self.copy_text("0"); } let val = self.expand_name_or_array_element(&rest[..=end])?; - return Ok(self.interp.shell_length(&val).to_string()); + return self.copy_text(&self.interp.shell_length(&val).to_string()); } let val = self.expand_variable(rest)?; - return Ok(self.interp.shell_length(&val).to_string()); + return self.copy_text(&self.interp.shell_length(&val).to_string()); } if let Some(bracket) = inner.find('[') @@ -1179,7 +1280,7 @@ impl ArithEval<'_, '_> { /// Expand a parameter expansion with operators inside arithmetic context. /// Handles common cases like ${var%%-*}, ${var##prefix}, etc. - fn expand_param_op_in_arithmetic(&mut self, inner: &str) -> ArithResult { + fn expand_param_op_in_arithmetic(&mut self, inner: &str) -> ArithResult { for (pos, ch) in inner.char_indices() { match ch { '%' => { @@ -1187,27 +1288,31 @@ impl ArithEval<'_, '_> { let value = self.expand_name_or_array_element(name)?; if inner[pos..].starts_with("%%") { let pattern = &inner[pos + 2..]; - return Ok(self.interp.remove_pattern(&value, pattern, false, true)); + return self + .copy_text(&self.interp.remove_pattern(&value, pattern, false, true)); } let pattern = &inner[pos + 1..]; - return Ok(self.interp.remove_pattern(&value, pattern, false, false)); + return self + .copy_text(&self.interp.remove_pattern(&value, pattern, false, false)); } '#' if pos > 0 => { let name = &inner[..pos]; let value = self.expand_name_or_array_element(name)?; if inner[pos..].starts_with("##") { let pattern = &inner[pos + 2..]; - return Ok(self.interp.remove_pattern(&value, pattern, true, true)); + return self + .copy_text(&self.interp.remove_pattern(&value, pattern, true, true)); } let pattern = &inner[pos + 1..]; - return Ok(self.interp.remove_pattern(&value, pattern, true, false)); + return self + .copy_text(&self.interp.remove_pattern(&value, pattern, true, false)); } ':' if inner[pos..].starts_with(":-") => { let name = &inner[..pos]; let default = &inner[pos + 2..]; let value = self.expand_name_or_array_element(name)?; if value.is_empty() { - return Ok(default.to_string()); + return self.copy_text(default); } return Ok(value); } @@ -1221,7 +1326,7 @@ impl ArithEval<'_, '_> { /// Used by parameter expansion inside arithmetic so `${arr[$key]:-N}` and /// friends can read associative/indexed array elements — `expand_variable` /// alone only handles scalar names. Fixes issue #1776. - fn expand_name_or_array_element(&mut self, name: &str) -> ArithResult { + fn expand_name_or_array_element(&mut self, name: &str) -> ArithResult { if let Some(bracket) = name.find('[') && name.ends_with(']') { @@ -1230,16 +1335,16 @@ impl ArithEval<'_, '_> { let idx_str = strip_subscript_quotes(&name[bracket + 1..name.len() - 1]); if let Some(arr) = self.interp.scoped.assoc_arrays.get(resolved) { let key = self.expand_variable_or_literal(idx_str)?; - return Ok(arr.get(&key).cloned().unwrap_or_default()); + return self.copy_text(arr.get(&*key).map_or("", String::as_str)); } if let Some(arr) = self.interp.scoped.arrays.get(resolved) { let idx = self.resolve_indexed_array_subscript(resolved, idx_str)?; - return Ok(arr.get(&idx).cloned().unwrap_or_default()); + return self.copy_text(arr.get(&idx).map_or("", String::as_str)); } if self.resolve_indexed_array_subscript(resolved, idx_str)? == 0 { return self.expand_variable(resolved); } - return Ok(String::new()); + return self.new_text(); } self.expand_variable(name) } @@ -1288,4 +1393,26 @@ mod tests { assert!(tokenize("a[1 ").is_err()); assert_eq!(tokenize("a[b[1]]+1").unwrap().len(), 4); } + #[test] + fn arithmetic_text_cap_and_cancellation_precede_growth() { + let mut interp = Interpreter::new(Arc::new(crate::InMemoryFs::new())); + interp.begin_execution_budget(); + let mut budget = ArithmeticBudget::new(); + let ev = ArithEval::new(&interp, &mut budget); + let mut out = ev.new_text().unwrap(); + let prefix = "é".repeat(32_768); + ArithEval::append_text(&interp, &mut out, &prefix).unwrap(); + assert_eq!(out.len(), 65_536); + assert_eq!( + ArithEval::append_text(&interp, &mut out, "x"), + Err("expression too long".to_string()) + ); + assert_eq!(&*out, prefix); + let mut cancelled = ev.new_text().unwrap(); + interp + .cancellation_token() + .store(true, std::sync::atomic::Ordering::Relaxed); + assert!(ArithEval::append_text(&interp, &mut cancelled, "x").is_err()); + assert!(cancelled.is_empty()); + } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index c310f91e8..10fce58d5 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -2926,7 +2926,7 @@ impl Interpreter { Arc::make_mut(&mut self.scoped.namerefs) } - /// Check if cancellation has been requested. + /// Check cancellation and resource failures recorded by read-only expansion. fn check_cancelled(&self) -> Result<()> { if let Some(error) = &self.memory_limit_error { return Err(crate::error::Error::ResourceLimit(error.clone())); @@ -2934,7 +2934,7 @@ impl Interpreter { if self.cancelled.load(Ordering::Relaxed) { Err(crate::error::Error::Cancelled) } else { - Ok(()) + self.execution_budget.check().map_err(Into::into) } } @@ -13146,57 +13146,70 @@ impl Interpreter { self.resolve_param_expansion_name(name).1 }) }; - Self::expand_key_text_using(s, lookup).unwrap() + let mut out = String::new(); + Self::scan_key_text(s, lookup, |text| { + out.push_str(text); + Ok(()) + }) + .unwrap(); + out } /// One quote/key scanner; callers own lookup semantics and recursion budgets. - fn expand_key_text_using( + fn scan_key_text, E>( s: &str, - mut lookup: impl FnMut(&str, bool) -> std::result::Result, - ) -> std::result::Result { - let mut out = String::new(); - let mut chars = s.chars().peekable(); - while let Some(c) = chars.next() { + mut lookup: impl FnMut(&str, bool) -> std::result::Result, + mut append: impl FnMut(&str) -> std::result::Result<(), E>, + ) -> std::result::Result<(), E> { + // Borrow names from the source: scanners must not allocate an + // unmetered temporary before a fallible expansion callback runs. + let mut chars = s.char_indices().peekable(); + while let Some((_, c)) = chars.next() { match c { '"' => {} '\'' => { - for q in chars.by_ref() { + for (_, q) in chars.by_ref() { if q == '\'' { break; } - out.push(q); + append(q.encode_utf8(&mut [0; 4]))?; + } + } + '\\' => { + if let Some((_, c)) = chars.next() { + append(c.encode_utf8(&mut [0; 4]))?; } } - '\\' => out.extend(chars.next()), - '$' if chars.peek() == Some(&'{') => { - chars.next(); - let mut name = String::new(); - for n in chars.by_ref() { + '$' if chars.peek().is_some_and(|(_, c)| *c == '{') => { + let (start, _) = chars.next().unwrap(); + let mut end = s.len(); + for (i, n) in chars.by_ref() { if n == '}' { + end = i; break; } - name.push(n); } - out.push_str(&lookup(&name, true)?); + append(&lookup(&s[start + 1..end], true)?)?; } '$' if chars .peek() - .is_some_and(|n| n.is_ascii_alphanumeric() || *n == '_') => + .is_some_and(|(_, n)| n.is_ascii_alphanumeric() || *n == '_') => { - let mut name = String::new(); - while let Some(&n) = chars.peek() { + let start = chars.peek().unwrap().0; + let mut end = start; + while let Some(&(i, n)) = chars.peek() { if !(n.is_ascii_alphanumeric() || n == '_') { break; } - name.push(n); + end = i + n.len_utf8(); chars.next(); } - out.push_str(&lookup(&name, false)?); + append(&lookup(&s[start..end], false)?)?; } - _ => out.push(c), + _ => append(c.encode_utf8(&mut [0; 4]))?, } } - Ok(out) + Ok(()) } /// Fully expand an associative array key using standard word expansion. diff --git a/crates/bashkit/tests/integration/arithmetic_resource_tests.rs b/crates/bashkit/tests/integration/arithmetic_resource_tests.rs index 4f108dc85..df6d81090 100644 --- a/crates/bashkit/tests/integration/arithmetic_resource_tests.rs +++ b/crates/bashkit/tests/integration/arithmetic_resource_tests.rs @@ -141,3 +141,132 @@ async fn dollar_subscript_writes_remain_read_only() { assert_eq!(result.stdout, "0 7\n"); assert!(result.stderr.is_empty()); } + +#[tokio::test] +async fn arithmetic_array_keys_obey_live_budget_and_recover() { + use bashkit::{Error, ExecutionLimits, LimitExceeded}; + for reference in ["$x", "${x}"] { + for expr in ["c[KEY]=1", "c[KEY]+=1", "c[KEY]++", "--c[KEY]", "c[KEY]"] { + for entry in ["((EXPR))", "let 'EXPR'", "echo $((EXPR))"] { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(512); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec(&format!("declare -A c; x={}", "A".repeat(64))) + .await + .unwrap(); + let expr = expr.replace("KEY", &reference.repeat(32)); + let script = entry.replace("EXPR", &expr); + let result = bash.exec(&script).await; + assert!( + matches!( + result, + Err(Error::ResourceLimit(LimitExceeded::ExecutionBudget(_))) + ), + "{script}: {result:?}" + ); + let recovery = bash.exec("echo recovered; echo ${#c[@]}").await.unwrap(); + assert_eq!(recovery.stdout, "recovered\n0\n"); + assert_eq!(recovery.exit_code, 0); + } + } + } +} + +#[tokio::test] +async fn arithmetic_array_key_scanner_obeys_budgets() { + use bashkit::{Error, ExecutionLimits, LimitExceeded}; + for expression in ["${c[k$x$x$x$x]}", "${c[k${x}${x}${x}${x}]}", "$r"] { + for limits in [ + ExecutionLimits::new().max_live_intermediate_bytes(512), + ExecutionLimits::new().max_work_units(400), + ] { + let mut bash = Bash::builder() + .limits(limits) + .env("x", "A".repeat(256)) + .build(); + bash.exec("declare -A c; declare -n r='c[k$x$x$x$x]'") + .await + .unwrap(); + let result = bash.exec(&format!("let '{expression}'")).await; + assert!( + matches!( + result, + Err(Error::ResourceLimit(LimitExceeded::ExecutionBudget(_))) + ), + "{expression}: {result:?}" + ); + assert_eq!( + bash.exec("echo recovered").await.unwrap().stdout, + "recovered\n" + ); + } + } +} + +#[tokio::test] +async fn arithmetic_array_key_size_boundary() { + for size in [65_535, 65_536] { + let mut bash = Bash::builder().env("x", "a".repeat(size)).build(); + bash.exec("declare -A c").await.unwrap(); + // The literal prefix makes the scanner's output exactly 64 KiB or + // one byte over. No huge arithmetic source or host-exhaustion probe. + let result = bash.exec("let '1+${c[k$x]:-0}'").await.unwrap(); + if size == 65_535 { + assert_eq!(result.exit_code, 0, "{}", result.stderr); + assert!(result.stderr.is_empty()); + } else { + assert_eq!(result.exit_code, 1); + assert!(result.stderr.contains("expression too long")); + } + assert_eq!( + bash.exec("echo recovered").await.unwrap().stdout, + "recovered\n" + ); + } + let mut bash = Bash::builder().env("x", "a".repeat(65_537)).build(); + // Inspecting length does not materialize that value as an expression/key. + let result = bash.exec("echo $((${#x}+1))").await.unwrap(); + assert_eq!(result.stdout, "65538\n"); +} + +#[tokio::test] +async fn arithmetic_array_keys_match_modern_bash() { + let version = std::process::Command::new("bash") + .args(["--noprofile", "--norc", "-c", "echo ${BASH_VERSINFO[0]}"]) + .output() + .unwrap(); + if String::from_utf8_lossy(&version.stdout) + .trim() + .parse::() + .unwrap() + < 4 + { + // Associative arrays require Bash >= 4; Linux CI supplies Bash 5.x. + return; + } + for script in [ + "declare -A c; x='a b'; ((c[$x$x]=3)); ((c[$x$x]+=2)); ((c[$x$x]++)); let 'c[$x$x]--'; echo $((c[$x$x])); echo ${c['a ba b']}", + "declare -A c; x=é; ((c[${x}${x}]=7)); echo $((c[${x}${x}])); let 'c[éé]+=1'; echo ${c[éé]}", + "declare -A c; c[abcabc]=7; x=abc; echo $((1+${c[k${x}${x}]:-2})); echo $((1+${c[$x$x]})); declare -n r='c[$x$x]'; echo $(($r))", + "declare -A c; a=(a b); c[k'a b']=8; echo $((1+${c[k${a[@]}]}))", + ] { + let expected = std::process::Command::new("bash") + .args(["--noprofile", "--norc", "-c", script]) + .output() + .unwrap(); + assert!( + expected.status.success(), + "{script}: {}", + String::from_utf8_lossy(&expected.stderr) + ); + let mut bash = Bash::new(); + let result = bash.exec(script).await.unwrap(); + assert_eq!( + result.exit_code, + expected.status.code().unwrap(), + "{script}: {}", + result.stderr + ); + assert_eq!(result.stdout.as_bytes(), expected.stdout, "{script}"); + assert!(result.stderr.is_empty(), "{script}: {}", result.stderr); + } +} diff --git a/docs/security.md b/docs/security.md index bce0d7ec0..27f1331ac 100644 --- a/docs/security.md +++ b/docs/security.md @@ -153,6 +153,12 @@ Arithmetic limits cover textual dollar expansion inside expressions. Nested array subscripts share the enclosing expression's recursion and work limits, including quoted arguments to `let`. Exceeding either limit returns an arithmetic error; subsequent executions can reuse the session. +Expanded arithmetic expressions and associative-array keys are capped at 64 KiB. +Their buffers reserve capacity against `max_live_intermediate_bytes` before +allocation, including temporary copies of variable values. Each append charges +`max_work_units` and checks cancellation and the execution deadline. Shared-budget +exhaustion returns a resource-limit error rather than continuing the script; +subsequent executions start with a fresh request budget. ## Panic safety diff --git a/knowledge/log.md b/knowledge/log.md index ca0bacc58..bd56d8da0 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -1,5 +1,9 @@ # Bashkit Knowledge Update Log +## 2026-10-11 + +* **Fix**: arithmetic dollar expansion and associative-key scanning reserve intermediate capacity before allocation, cap generated expression/key bytes at 64 KiB before append, and charge shared work with synchronous cancellation/deadline checks. Scalar storage is borrowed before budgeted copying; temporary value buffers keep their leases until consumed. Poisoned request budgets propagate through command boundaries as resource-limit errors. Small-budget regressions cover arithmetic entry points, read/write operations, key scanning, namerefs, size boundaries, recovery, and finite Bash parity. See [threat model](security/threat-model.md) TM-DOS-026 and [security testing](security/security-testing.md). + ## 2026-10-09 * **Test**: first run of the tagged 88-task `bashkit_bash` and `bashkit_generate` on five models (Opus 5.5 87/88, Kimi K3 86, Sonnet 5.5 85, Muse Spark 84, Gemini 3.8 Flash 74; generate 13-15/15); GPT-6 models pending (OpenAI credit). The homepage eval table now only lists runs over the newest run's task count, so scores on different dataset sizes never mix. Results in `crates/bashkit-eval/README.md`. diff --git a/knowledge/security/security-testing.md b/knowledge/security/security-testing.md index 0b4cdf4eb..efa2e4547 100644 --- a/knowledge/security/security-testing.md +++ b/knowledge/security/security-testing.md @@ -96,6 +96,20 @@ orphan entries or retained filename allocation. No host-exhaustion probe is need The real-Bash comparison sources both the original and post-unset replacement; Bash versions differ in the filename they assign to a definition inside `-c`. +## Arithmetic Associative-Key Budgets + +`arithmetic_resource_tests` exercises the public `Bash::exec` path with keys +bounded to a few KiB and small live-memory/work budgets. It covers repeated +`$name` and `${name}` through `((...))`, quoted `let`, and `$((...))`, reads, +assignment/compound assignment/increment/decrement, recursive key scanning, +namerefs, 64 KiB key boundaries, and session recovery after rejection. Finite +valid-key cases compare stdout and status with Bash >= 4 (associative arrays +are unavailable in macOS's stock Bash 3.2; Linux CI supplies Bash 5.x). +The interpreter unit test checks the exact UTF-8 append boundary and a +cancellation checkpoint without growing a buffer after rejection. Resource +probes run in externally supervised subprocesses; full memory-exhaustion +payloads are unnecessary. + ## Adding New Fail Points Add `fail_point!("module::function", |action| ...)` under diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index c3fb3420f..84263f78f 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -240,7 +240,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-020 | Function and signal-trap recursion | `f() { f; }; f` or `trap 'kill -USR1 $$' USR1; kill -USR1 $$` | Signal handlers, functions, source, and child shells share the function-depth counter, plus a dedicated trap-nesting cap of 3 that trips first on small stacks; direct, mutual, and mixed signal recursion cannot refresh either. Nested-trap exhaustion is skipped with a diagnostic and the first trip fails the exec as a resource-limit error, while handler logic failures never disturb the interrupted command (TM-DOS-030). Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). CLI subprocess tests run direct, mutual, and mixed signal/function/source recursion on that stack size with small command budgets. | **MITIGATED** | | TM-DOS-021 | Command sub nesting | `$($($($())))` | Child parsers increment shared AST depth and inherit ceiling, fuel, budget and timeout origin; heap ownership and outlined nonrecursive frames keep the hard cap stack-safe | **MITIGATED** | | TM-DOS-022 | Parser recursion | Deeply nested `(((())))` | `max_ast_depth` limit (100) + `HARD_MAX_AST_DEPTH` cap (100) | **MITIGATED** | -| TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))`, nested `${a[${a[...]}]}` passed to `let` | `MAX_ARITHMETIC_DEPTH` limit (50) checked before dollar expansion and tokenization; shared fuel across textual expansion, nested read-only subscripts, nameref/key lookup, and recursive variable evaluation; expanded-size cap | **MITIGATED** | +| TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))`, nested `${a[${a[...]}]}` passed to `let` | `MAX_ARITHMETIC_DEPTH` limit (50) checked before dollar expansion and tokenization; shared fuel across textual expansion, nested read-only subscripts, nameref/key lookup, and recursive variable evaluation; generated expression/key bytes capped at 64 KiB before append; `BudgetedString` leases capacity and temporary value copies before allocation; appends charge shared work and check cancellation/deadline; command boundaries propagate poisoned shared budgets as resource-limit errors. Regressions: `arithmetic_resource_tests`, `arithmetic_text_cap_and_cancellation_precede_growth` | **MITIGATED** | | TM-DOS-064 | Heredoc suffix re-injection CPU amplification | Many `: <