diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 30e07a1d0..152abca7b 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -308,7 +308,7 @@ Scripts may attempt to leak sensitive information. | Threat | Attack Example | Mitigation | Status | |--------|---------------|------------|--------| -| Library Debug shapes leak via stderr (TM-INF-022) | `{:?}` dumps internal struct shapes into agent-visible stderr | Static scan forbids Debug formatting in builtins, plus per-tool leak tests and fuzz invariants | MITIGATED | +| Library Debug shapes leak via stderr (TM-INF-022) | `{:?}` dumps internal struct shapes into agent-visible stderr; separately, a diagnostic that quotes the script back can run past the 1 KB stderr budget — an arithmetic error named both the whole expression and the whole unparsed rest, echoing one input back twice over | Static scan forbids Debug formatting in builtins, plus per-tool leak tests and fuzz invariants. Each run of script text echoed into an arithmetic diagnostic is capped (`MAX_ARITHMETIC_DIAG_ECHO`), so the text that says what went wrong always survives (L-ARITH-002) | MITIGATED | | jq `halt`/`halt_error` exits the host process (TM-INF-023) | Filter calls `halt(N)` → `std::process::exit` | Upstream `halt` native stripped; safe replacement returns a jq error | MITIGATED | | Host env side-channel via clap `Arg::env` (TM-INF-024) | `ls` resolved `TABSIZE`/`TIME_STYLE` from host env | Codegen strips `.env(...)`; builtins read `ctx.env` only | MITIGATED | | Untrusted generated Rust in drift CI (TM-INF-025) | Malicious upstream `uu_app()` runs with a write token | Generator validates the emitted shape; drift workflow splits read/write privilege | FIXED | diff --git a/crates/bashkit/src/interpreter/arithmetic.rs b/crates/bashkit/src/interpreter/arithmetic.rs index f0c679637..dc8838019 100644 --- a/crates/bashkit/src/interpreter/arithmetic.rs +++ b/crates/bashkit/src/interpreter/arithmetic.rs @@ -23,6 +23,23 @@ use super::*; /// key for associative arrays. pub(super) type ArithWrite = (String, Option, String); +/// Marker appended where [`diag_echo`] cut source text short. +pub(super) const TRUNCATION_MARKER: &str = "..."; + +/// Bound a run of script text that is about to be named in a diagnostic. +/// +/// THREAT[TM-INF-022]: arithmetic errors echo the expression and the unparsed +/// rest, both straight from the script. Real bash prints them whole, which for +/// a long expression puts the diagnostic over bashkit's 1 KiB budget (L-ARITH-002); +/// truncating each fragment keeps the explanatory text that follows it. +pub(super) fn diag_echo(src: &str) -> std::borrow::Cow<'_, str> { + if src.len() <= Interpreter::MAX_ARITHMETIC_DIAG_ECHO { + return std::borrow::Cow::Borrowed(src); + } + let end = src.floor_char_boundary(Interpreter::MAX_ARITHMETIC_DIAG_ECHO); + std::borrow::Cow::Owned(format!("{}{TRUNCATION_MARKER}", &src[..end])) +} + #[derive(Debug, Clone, PartialEq)] enum Tok { Num(String), @@ -236,7 +253,7 @@ impl ArithParser<'_> { if rest.is_empty() { msg.to_string() } else { - format!("{msg} (error token is \"{rest}\")") + format!("{msg} (error token is \"{}\")", diag_echo(rest)) } } } @@ -279,7 +296,7 @@ impl<'a> ArithEval<'a> { } let toks = tokenize(src).map_err(|(msg, at)| { let rest = src[at..].trim(); - format!("{msg} (error token is \"{rest}\")") + format!("{msg} (error token is \"{}\")", diag_echo(rest)) })?; let mut p = ArithParser { src, toks, pos: 0 }; self.enter()?; @@ -587,8 +604,8 @@ impl<'a> ArithEval<'a> { Ok(v) } Tok::Num(s) => { - let v = - parse_arith_number(&s).map_err(|m| format!("{m} (error token is \"{s}\")"))?; + let v = parse_arith_number(&s) + .map_err(|m| format!("{m} (error token is \"{}\")", diag_echo(&s)))?; p.next(); Ok(v) } @@ -647,7 +664,10 @@ impl<'a> ArithEval<'a> { return Ok(0); } let token = p.src.get(rhs_at..).unwrap_or("").trim(); - return Err(format!("division by 0 (error token is \"{token}\")")); + return Err(format!( + "division by 0 (error token is \"{}\")", + diag_echo(token) + )); } if op == "/" { l.wrapping_div(r) @@ -835,7 +855,7 @@ impl Interpreter { pub(super) fn try_evaluate_arithmetic_with_assign(&mut self, expr: &str) -> ArithResult { let (r, writes) = self.arith_eval(expr); self.apply_arith_writes(writes); - r.map_err(|m| format!("{}: {m}", expr.trim())) + r.map_err(|m| format!("{}: {m}", diag_echo(expr.trim()))) } /// Evaluate with side effects; an error is recorded for the command @@ -856,7 +876,7 @@ impl Interpreter { match self.arith_eval(expr).0 { Ok(v) => v, Err(msg) => { - self.record_arith_error(format!("{}: {msg}", expr.trim())); + self.record_arith_error(format!("{}: {msg}", diag_echo(expr.trim()))); 0 } } diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index 0b7146176..cfd3720f0 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2725,9 +2725,13 @@ impl Interpreter { Some(expr) => { let n = self.evaluate_arithmetic(expr); if n < 0 { - return Err( - self.arith_diag("", &format!("{}: substring expression < 0", expr.trim())) - ); + return Err(self.arith_diag( + "", + &format!( + "{}: substring expression < 0", + arithmetic::diag_echo(expr.trim()) + ), + )); } usize::try_from(n).unwrap_or(usize::MAX) } @@ -2780,9 +2784,13 @@ impl Interpreter { (true, Some(s), Some(e)) if e >= s => return Ok((s, e)), (true, None, _) => return Ok((0, 0)), _ => { - return Err( - self.arith_diag("", &format!("{}: substring expression < 0", expr.trim())) - ); + return Err(self.arith_diag( + "", + &format!( + "{}: substring expression < 0", + arithmetic::diag_echo(expr.trim()) + ), + )); } } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index bd50c7a97..0ec20f358 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -6053,7 +6053,7 @@ impl Interpreter { match r { Ok(v) => *slot = v, Err(msg) => { - let msg = format!("{}: {msg}", operand.trim()); + let msg = format!("{}: {msg}", arithmetic::diag_echo(operand.trim())); let diag = self.arith_diag("[[: ", &msg); self.cond_stderr.push_str(&diag); return false; @@ -13035,6 +13035,14 @@ impl Interpreter { /// Maximum expanded arithmetic expression size accepted before fallback to 0. /// THREAT[TM-DOS-026]: Prevents attacker-controlled multi-megabyte arithmetic strings. const MAX_ARITHMETIC_EXPANSION_BYTES: usize = 64 * 1024; + /// Longest run of source text echoed into one arithmetic diagnostic. + /// THREAT[TM-INF-022]: an arithmetic error names both the whole expression + /// and the unparsed rest as the "error token". Both come from the script, + /// so an expression just under `MAX_ARITHMETIC_EXPANSION_BYTES` rendered a + /// diagnostic about twice that size. Capping each echoed fragment keeps the + /// line inside the 1 KiB diagnostic budget while leaving room for the fixed + /// text that says what actually went wrong. See L-ARITH-002. + const MAX_ARITHMETIC_DIAG_ECHO: usize = 256; /// Expand a string as a variable reference, or return as literal. /// Used for associative array keys which may be variable refs or literals. diff --git a/crates/bashkit/tests/integration/arithmetic_fuzz_scaffold_tests.rs b/crates/bashkit/tests/integration/arithmetic_fuzz_scaffold_tests.rs new file mode 100644 index 000000000..3f158d2ac --- /dev/null +++ b/crates/bashkit/tests/integration/arithmetic_fuzz_scaffold_tests.rs @@ -0,0 +1,91 @@ +// Scaffold tests for the arithmetic_fuzz target. +// +// The target wraps its input in `echo $((...))` and asserts the fuzz +// invariants: no panic, and stderr that neither leaks Debug shapes/host paths +// nor runs past `bashkit::testing::MAX_STDERR_BYTES`. +// +// An arithmetic error names the expression and then the unparsed rest as the +// "error token". Both come from the script, so an expression whose first +// rejected character sits near the front is echoed back roughly twice -- +// nightly fuzz run 270 turned 507 bytes of input into 1,076 bytes of stderr. +// `Interpreter::MAX_ARITHMETIC_DIAG_ECHO` bounds each fragment (L-ARITH-002). + +use bashkit::testing::{fuzz_exec, fuzz_init}; +use bashkit::{Bash, ExecutionLimits}; + +/// The limits `fuzz_targets/arithmetic_fuzz.rs` builds. +fn fuzz_bash() -> Bash { + fuzz_init(); + Bash::builder() + .limits( + ExecutionLimits::new() + .max_commands(100) + .max_function_depth(10) + .max_subst_depth(5) + .max_stdout_bytes(4096) + .max_stderr_bytes(4096) + .timeout(std::time::Duration::from_millis(100)), + ) + .build() +} + +async fn fuzz_arith(expr: &str, ctx: &str) { + let mut bash = fuzz_bash(); + fuzz_exec(&mut bash, &format!("echo $(({expr}))"), ctx, &[]).await; +} + +/// The exact crash input from nightly fuzz run 270. +const RUN_270_CRASH: &str = "+---+~~~~~~~~~~~#~~~~~~~~ech~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#~~~~~~~~ech~~\ + ~~~~~~~~~~~~~~~~~~~~~u~~~~~~~~~~|~~_LOWER_~~~~~~~~~~~~~~~~~~~~#~~~~~~~~e\ + ch~~~~~~~~~~~~~~~~~~~~~~~u~~~~~~~~~~|~~_LOWER_~~~~~~~~~~~~~~~~~~~~~~~~~~\ + ~~~~u~~~~~~~~~~~~~~~~~~~~~u~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~u~~~~~~~~~~\ + ~~~~~~~~--~~~u~~~~~~~~~~|~~_LOWER_~~~~~~~~~~~~~~~~~~~~#~~~~~~~~ech~~~~~~\ + ~~~~~~~~~~~~~~~~~u~~~~~~~~~~|~~_LOWER_~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~u~~~\ + ~~~~~~~~~~~~~~~~~~u~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~u~~~~~~~~~~~~~~~~~~\ + ---"; + +#[tokio::test] +async fn run_270_crash_input_is_bounded() { + // The literal is line-continued above; check it still reassembles to the + // 507 bytes libFuzzer reported, so a reflow cannot quietly shrink it. + assert_eq!(RUN_270_CRASH.len(), 507); + fuzz_arith(RUN_270_CRASH, "run_270_crash_input").await; +} + +#[tokio::test] +async fn long_rejected_operator_near_front() { + // Reduced shape of the same bug: one rejected byte, a long tail. + fuzz_arith(&format!("~#{}", "~".repeat(500)), "long_rejected_operator").await; +} + +#[tokio::test] +async fn long_number_token() { + fuzz_arith(&format!("1{}x", "9".repeat(600)), "long_number_token").await; +} + +#[tokio::test] +async fn long_division_by_zero_token() { + fuzz_arith(&format!("1/(0{})", "+0".repeat(400)), "long_division_token").await; +} + +#[tokio::test] +async fn long_unary_run_hits_the_depth_limit() { + fuzz_arith(&"~".repeat(500), "long_unary_run").await; +} + +#[tokio::test] +async fn deeply_grouped_expression() { + let depth = 200; + fuzz_arith( + &format!("{}1{}", "(".repeat(depth), ")".repeat(depth)), + "deeply_grouped", + ) + .await; +} + +#[tokio::test] +async fn valid_expression_still_evaluates() { + let mut bash = fuzz_bash(); + let r = bash.exec("echo $((2 + 3 * 4))").await.unwrap(); + assert_eq!(r.stdout, "14\n"); +} diff --git a/crates/bashkit/tests/integration/diagnostic_prefix_tests.rs b/crates/bashkit/tests/integration/diagnostic_prefix_tests.rs index 6f3abd0f3..5c429ad85 100644 --- a/crates/bashkit/tests/integration/diagnostic_prefix_tests.rs +++ b/crates/bashkit/tests/integration/diagnostic_prefix_tests.rs @@ -231,3 +231,119 @@ async fn interactive_mode_keeps_bare_prefix() { let r = bash.exec("nocmd").await.unwrap(); assert_eq!(r.stderr, "bash: nocmd: command not found\n"); } + +// --- Arithmetic diagnostics stay inside the diagnostic budget (TM-INF-022) --- +// +// `$(( ))` echoes two attacker-controlled fragments into one line: the whole +// expression, and the unparsed rest as the "error token". Both were unbounded, +// so one bounded expression produced a diagnostic about twice its size. +// `arithmetic_fuzz` (nightly fuzz run 270) found a 507-byte expression that +// rendered 1,076 bytes of stderr, over the 1 KiB cap +// `bashkit::testing::assert_no_leak` holds every builtin to. + +/// Same cap as `bashkit::testing::MAX_STDERR_BYTES`. +const MAX_DIAG: usize = 1024; + +/// The reduced `arithmetic_fuzz` crash. The rejected `#` sits near the front, +/// so the unparsed rest -- the "error token" -- is nearly as long as the +/// expression echoed before it. That doubling is what went over the cap. +fn long_bad_expression() -> String { + format!("~#{}", "~".repeat(500)) +} + +#[tokio::test] +async fn long_arithmetic_expansion_diagnostic_is_bounded() { + let (err, code) = run_c(&format!("echo $(({}))", long_bad_expression())).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + // Truncation keeps the part that says what went wrong. + assert!(err.starts_with("bash: line 1: "), "{err}"); + assert!(err.contains("syntax error"), "{err}"); + assert_eq!(code, 1); +} + +#[tokio::test] +async fn long_arithmetic_command_diagnostic_is_bounded() { + let (err, code) = run_c(&format!("(({}))", long_bad_expression())).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + assert!(err.contains("syntax error"), "{err}"); + assert_eq!(code, 1); +} + +#[tokio::test] +async fn long_let_diagnostic_is_bounded() { + let (err, code) = run_c(&format!("let '{}'", long_bad_expression())).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + assert!(err.contains("syntax error"), "{err}"); + assert_eq!(code, 1); +} + +#[tokio::test] +async fn long_cond_arithmetic_diagnostic_is_bounded() { + let (err, _) = run_c(&format!("[[ {} -eq 1 ]]", long_bad_expression())).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + assert!(err.contains("syntax error"), "{err}"); +} + +#[tokio::test] +async fn long_division_by_zero_diagnostic_is_bounded() { + // The error token here is the right-hand side, echoed from the source. + let (err, code) = run_c(&format!("echo $((1/(0{}) ))", "+0".repeat(400))).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + assert!(err.contains("division by 0"), "{err}"); + assert_eq!(code, 1); +} + +#[tokio::test] +async fn long_bad_number_diagnostic_is_bounded() { + let (err, code) = run_c(&format!("echo $((1{}x))", "9".repeat(600))).await; + assert!( + err.len() <= MAX_DIAG, + "stderr is {} bytes:\n{err}", + err.len() + ); + assert_eq!(code, 1); +} + +#[tokio::test] +async fn short_arithmetic_diagnostics_are_not_truncated() { + // The common case stays byte for byte with bash 5.2. + let (err, code) = run_c("echo $((1/0))").await; + assert_eq!( + err, + "bash: line 1: 1/0: division by 0 (error token is \"0\")\n" + ); + assert_eq!(code, 1); + + let (err, code) = run_c("echo $((1/(0+0)))").await; + assert_eq!( + err, + "bash: line 1: 1/(0+0): division by 0 (error token is \"(0+0)\")\n" + ); + assert_eq!(code, 1); + + // An expression under the cap is still echoed whole, with no marker. + let expr = format!("{}#x", "~".repeat(100)); + let (err, _) = run_c(&format!("echo $(({expr}))")).await; + assert!(err.contains(&expr), "{err}"); + assert!(!err.contains("..."), "{err}"); +} diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index 9640c9f0f..8d649db77 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -16,6 +16,7 @@ pub mod agent_skills_publication_tests; pub mod allexport_tests; pub mod archive_bzip2_tests; +pub mod arithmetic_fuzz_scaffold_tests; pub mod array_budget_security_tests; pub mod awk_fuzz_scaffold_tests; pub mod awk_newline_tests; diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index a67ce52f9..5481bd635 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -59,6 +59,7 @@ execution model. Evidence is a threat-model ID, a test, or `stance` | L-NET-001 | No raw network sockets; HTTP only via `curl`/`wget`/`http` builtins | Allowlist-mediated egress is the only network surface | `l_net_001_no_raw_sockets` | | L-NET-002 | No DNS resolution; hosts must appear in the allowlist | Resolution would bypass allowlist intent | `l_net_002_default_deny_no_resolution` | | L-ARITH-001 | A variable's value read as an arithmetic expression or subscript never runs command substitution: `x='a[$(cmd)]'; echo $((x))` and `unset "a[$(cmd)]"` built from data are arithmetic syntax errors, and an indirect target holding `$(`, `` ` ``, `<(` or `>(` (`r='a[$(cmd)]'; ${!r}`) is `invalid variable name`, where bash runs `cmd` | Values are data; re-parsing them as code is the classic bash arithmetic injection, and a sandboxed agent shell must not execute text it only read | stance | +| L-ARITH-002 | An arithmetic error names at most 256 bytes each of the expression and the "error token" (the unparsed rest), with a `...` marker where either was cut. An expression whose first rejected character sits near the front is echoed back twice over -- once whole, once from that character on -- so a ~500-byte one reported ~1,000 bytes before this cap, as bash still does. Expressions at or under the cap are byte-identical to bash | Both fragments come from the script, so one expression under `MAX_ARITHMETIC_EXPANSION_BYTES` rendered a diagnostic about twice its size, over the 1 KiB stderr budget every builtin is held to (TM-INF-022). Capping each fragment keeps the text that says what went wrong | TM-INF-022, `long_arithmetic_expansion_diagnostic_is_bounded`, `run_270_crash_input_is_bounded` | | L-SIG-001 | No signal arrives from outside the sandbox, so a handler only runs for a signal the script sends itself (`kill -SIG $$`, plus EXIT, ERR and DEBUG). Without a handler, a self-sent signal ends the script with 128 + signal | No host signals exist inside the sandbox | `l_sig_001_signal_traps_not_delivered` | | L-SSH-001 | CA-signed SSH host *certificates* are rejected under `strict_host_key_checking` (default), even when the public key they wrap is a configured trusted key; configure the host's public key directly | No CA trust store exists to validate the signature chain, validity window, principals or critical options; matching the embedded key would grant trust never actually verified | TM-SSH-006, `test_strict_rejects_certificate_even_when_inner_key_is_trusted` | | L-WASM-001 | **Removed:** JS-host timers now drive `sleep`, builtin `timeout`, execution limits, and tool `timeoutMs` | `gloo-timers` bridges the host event-loop clock without threads or cross-origin isolation | [Browser Package](../runtimes/browser-package.md) | diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 636fb7431..14fa3660c 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -483,6 +483,17 @@ then generalized via the static + dynamic + fuzz guards in the table. New builti library errors must use Display (`{}`) or a domain formatter, reference shape: `format_compile_errors` in `builtins/jq/errors.rs`. +The 1 KB ceiling also binds diagnostics the *interpreter* writes, not just +builtins wrapping libraries. Arithmetic errors named the whole expression and +the whole unparsed rest (the "error token"), both straight from the script, so +one expression under `MAX_ARITHMETIC_EXPANSION_BYTES` rendered a diagnostic +about twice its size -- `arithmetic_fuzz` (nightly fuzz run 270) tripped +`assert_no_leak` with 1,076 bytes from a 507-byte expression. Each echoed +fragment is now bounded by `MAX_ARITHMETIC_DIAG_ECHO` via +`interpreter::arithmetic::diag_echo`, which keeps the fixed explanatory text +that follows it (L-ARITH-002). Any new diagnostic that quotes script text back +has the same obligation: bound the quoted run, not the finished line. + Display of a **Bashkit** error is not a safe formatter either: `Error`'s own variants stringify as Rust enum shapes (`io error: `, `internal error: `) that no shell prints, and `internal error:` is in `UNIVERSAL_BANNED`. Builtins