Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 74 additions & 51 deletions crates/bashkit/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6160,7 +6160,75 @@ impl Interpreter {
}
}

async fn expand_word(&mut self, word: &Word) -> Result<String> {
// THREAT[TM-DOS-089]: Command substitution body extracted into a Box::pin-ed
// helper to cap per-level stack usage. Without this, each $(...) nesting level
// adds the full expand_word state machine to the call stack, causing overflow
// at moderate depths despite the logical depth limit.
fn execute_cmd_subst<'a>(
&'a mut self,
commands: &'a [Command],
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<String>> + Send + 'a>> {
Box::pin(async move {
// Command substitution runs in a subshell: snapshot all
// mutable state so mutations don't leak to the parent.
let saved_traps = self.traps.clone();
let saved_functions = self.functions.clone();
let saved_vars = self.variables.clone();
let saved_arrays = self.arrays.clone();
let saved_assoc = self.assoc_arrays.clone();
let saved_aliases = self.aliases.clone();
let saved_cwd = self.cwd.clone();
let saved_memory_budget = self.memory_budget.clone();
let mut stdout = String::new();
for cmd in commands {
let cmd_result = self.execute_command(cmd).await?;
stdout.push_str(&cmd_result.stdout);
self.last_exit_code = cmd_result.exit_code;
if matches!(cmd_result.control_flow, ControlFlow::Exit(_)) {
break;
}
}
// Fire EXIT trap set inside the command substitution
if let Some(trap_cmd) = self.traps.get("EXIT").cloned()
&& saved_traps.get("EXIT") != Some(&trap_cmd)
&& let Ok(trap_script) = Parser::with_limits(
&trap_cmd,
self.limits.max_ast_depth,
self.limits.max_parser_operations,
)
.parse()
&& let Ok(trap_result) = self.execute_command_sequence(&trap_script.commands).await
{
stdout.push_str(&trap_result.stdout);
}
// Restore parent state
self.traps = saved_traps;
self.functions = saved_functions;
self.variables = saved_vars;
self.arrays = saved_arrays;
self.assoc_arrays = saved_assoc;
self.aliases = saved_aliases;
self.cwd = saved_cwd;
self.memory_budget = saved_memory_budget;
self.counters.pop_subst();
self.subst_generation += 1;
let trimmed = stdout.trim_end_matches('\n');
Ok(trimmed.to_string())
})
}

// THREAT[TM-DOS-089]: Box::pin the expand_word future to cap per-level
// stack usage. Without this, the async state machine of expand_word (which
// contains all WordPart match arms) is inlined into the caller's future,
// causing stack overflow at moderate command substitution depths.
fn expand_word<'a>(
&'a mut self,
word: &'a Word,
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<String>> + Send + 'a>> {
Box::pin(async move { self.expand_word_inner(word).await })
}

async fn expand_word_inner(&mut self, word: &Word) -> Result<String> {
let mut result = String::new();
let mut is_first_part = true;

Expand Down Expand Up @@ -6212,61 +6280,16 @@ impl Interpreter {
}
}
WordPart::CommandSubstitution(commands) => {
// THREAT[TM-DOS-088]: Track substitution depth separately from
// function depth. Each level clones the full interpreter state,
// so memory ≈ depth × state_size. A tighter limit than
// max_function_depth prevents OOM.
// THREAT[TM-DOS-088]: Track substitution depth to prevent OOM.
if self.counters.push_subst(&self.limits).is_err() {
return Err(crate::error::Error::Execution(
"maximum command substitution depth exceeded".to_string(),
));
}
// Command substitution runs in a subshell: snapshot all
// mutable state so mutations don't leak to the parent.
let saved_traps = self.traps.clone();
let saved_functions = self.functions.clone();
let saved_vars = self.variables.clone();
let saved_arrays = self.arrays.clone();
let saved_assoc = self.assoc_arrays.clone();
let saved_aliases = self.aliases.clone();
let saved_cwd = self.cwd.clone();
let saved_memory_budget = self.memory_budget.clone();
let mut stdout = String::new();
for cmd in commands {
let cmd_result = self.execute_command(cmd).await?;
stdout.push_str(&cmd_result.stdout);
self.last_exit_code = cmd_result.exit_code;
if matches!(cmd_result.control_flow, ControlFlow::Exit(_)) {
break;
}
}
// Fire EXIT trap set inside the command substitution
if let Some(trap_cmd) = self.traps.get("EXIT").cloned()
&& saved_traps.get("EXIT") != Some(&trap_cmd)
&& let Ok(trap_script) = Parser::with_limits(
&trap_cmd,
self.limits.max_ast_depth,
self.limits.max_parser_operations,
)
.parse()
&& let Ok(trap_result) =
self.execute_command_sequence(&trap_script.commands).await
{
stdout.push_str(&trap_result.stdout);
}
// Restore parent state
self.traps = saved_traps;
self.functions = saved_functions;
self.variables = saved_vars;
self.arrays = saved_arrays;
self.assoc_arrays = saved_assoc;
self.aliases = saved_aliases;
self.cwd = saved_cwd;
self.memory_budget = saved_memory_budget;
self.counters.pop_subst();
self.subst_generation += 1;
let trimmed = stdout.trim_end_matches('\n');
result.push_str(trimmed);
// THREAT[TM-DOS-089]: Delegate to Box::pin-ed helper to
// prevent stack growth proportional to nesting depth.
let trimmed = self.execute_cmd_subst(commands).await?;
result.push_str(&trimmed);
}
WordPart::ArithmeticExpansion(expr) => {
let expanded_expr = if expr.contains("$(") {
Expand Down
58 changes: 58 additions & 0 deletions crates/bashkit/tests/stack_overflow_regression_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// THREAT[TM-DOS-089]: Stack overflow regression tests for nested command substitution.
// Before the Box::pin fix, each $(...) level inlined its async state machine into the
// caller's stack frame, causing overflow at moderate depths. The fix moves the command
// substitution body to the heap via Box::pin, keeping stack usage constant per level.

use bashkit::{Bash, ExecutionLimits};

/// Default max_subst_depth (32) must not cause stack overflow.
/// Before the Box::pin fix, this would SIGABRT at ~20-30 levels.
/// Regression test for issue #1089.
#[tokio::test]
async fn depth_32_no_stack_overflow() {
let depth = 32;
let mut cmd = "echo hello".to_string();
for _ in 0..depth {
cmd = format!("echo $({})", cmd);
}

let mut bash = Bash::builder()
.limits(
ExecutionLimits::new()
.max_commands(500)
.max_subst_depth(32)
.timeout(std::time::Duration::from_secs(5)),
)
.build();

// Must not stack-overflow — should return a result (possibly truncated by depth limit)
let result = bash.exec(&cmd).await;
if let Ok(r) = result {
// Depth limit error is also acceptable (Err case)
assert!(!r.stdout.is_empty() || r.exit_code != 0);
}
}

/// Deeply nested $() in arithmetic context (the fuzz crash vector).
/// Regression test for the specific #1089 crash input pattern.
#[tokio::test]
async fn nested_subst_in_arithmetic_no_overflow() {
// Simulates the crash input: nested $( inside $((...))
let mut inner = "echo 1".to_string();
for _ in 0..15 {
inner = format!("echo $({})", inner);
}
let script = format!("echo $(($({})))", inner);

let mut bash = Bash::builder()
.limits(
ExecutionLimits::new()
.max_commands(500)
.max_subst_depth(20)
.timeout(std::time::Duration::from_secs(5)),
)
.build();

// Must not panic or SIGABRT
let _ = bash.exec(&script).await;
}
1 change: 1 addition & 0 deletions specs/006-threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
| TM-INT-007 | `/dev/urandom` empty with `head -c` | Weak randomness — `head -c 16 /dev/urandom` returns empty string | Fix virtual device pipe handling in head builtin |
| TM-DOS-044 | Nested `$()` stack overflow (regression) | Process crash (SIGABRT) at depth ~50 despite #492 fix | Interpreter execution path may need separate depth tracking from lexer fix |
| TM-DOS-088 | Command substitution OOM via state cloning | OOM at depth N (memory ≈ N × state_size) | Dedicated `max_subst_depth` limit (default 32), separate from `max_function_depth` — **FIXED** via #1088 |
| TM-DOS-089 | Command substitution stack overflow via inlined futures | SIGABRT at ~20-30 nested $() levels | Box::pin `expand_word` and `execute_cmd_subst` to cap per-level stack — **FIXED** via #1089 |

### Accepted (Low Priority)

Expand Down
Loading