fix(security): bound tar lists and nested substitution parsing

crates/bashkit-cpython-wasm/src/lib.rs

@@ -30,17 +30,58 @@ pub const STDLIB_PREFIX: &str = "/usr/local/lib";
 /// Stdlib sources, deflate-compressed zip read by CPython's `zipimport`.
 pub static STDLIB_ZIP: &[u8] = include_bytes!("../artifacts/python314.zip");
 
-// Wasmtime maps the module's data segments copy-on-write straight from these
-// bytes, which requires host-page alignment. 64 KiB covers every supported
-// page size (4 KiB x86-64, 16 KiB Apple silicon, 64 KiB some aarch64 Linux).
+// Wasmtime maps data segments from immutable host-page-aligned bytes. Keep
+// 64 KiB alignment for every supported host. Mach-O only preserves host-page
+// alignment through linking/ASLR; do not form an over-aligned Rust reference
+// there. Cache a padded, aligned copy once when the embedded slice needs one.
+const MODULE_ALIGNMENT: usize = 65536;
+
+#[cfg(not(target_vendor = "apple"))]
 #[repr(C, align(65536))]
 struct Aligned<T: ?Sized>(T);
 
+#[cfg(not(target_vendor = "apple"))]
 static CWASM: &Aligned<[u8]> = &Aligned(*include_bytes!(concat!(env!("OUT_DIR"), "/python.cwasm")));
+#[cfg(target_vendor = "apple")]
+static CWASM: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/python.cwasm"));
+
+struct PaddedModule {
+    // Retain the Vec allocation: converting to a boxed slice can relocate it
+    // after we chose the aligned offset. No mutation is exposed after copying.
+    storage: Vec<u8>,
+    start: usize,
+    len: usize,
+}
+
+impl PaddedModule {
+    fn new(bytes: &[u8]) -> Self {
+        let mut storage = vec![0; bytes.len() + MODULE_ALIGNMENT - 1];
+        let start = storage.as_ptr().align_offset(MODULE_ALIGNMENT);
+        storage[start..start + bytes.len()].copy_from_slice(bytes);
+        Self {
+            storage,
+            start,
+            len: bytes.len(),
+        }
+    }
+
+    fn bytes(&self) -> &[u8] {
+        &self.storage[self.start..self.start + self.len]
+    }
+}
 
 /// Precompiled module bytes (Pulley, or native with the `native` feature) (for diagnostics and size reporting).
+/// The returned slice is immutable and 64 KiB aligned for the process lifetime.
 pub fn cwasm_bytes() -> &'static [u8] {
-    &CWASM.0
+    #[cfg(not(target_vendor = "apple"))]
+    let bytes: &[u8] = &CWASM.0;
+    #[cfg(target_vendor = "apple")]
+    let bytes = CWASM;
+    if bytes.as_ptr().align_offset(MODULE_ALIGNMENT) == 0 {
+        return bytes;
+    }
+    static PADDED: std::sync::OnceLock<PaddedModule> = std::sync::OnceLock::new();
+    PADDED.get_or_init(|| PaddedModule::new(bytes)).bytes()
 }
 
 /// Create an engine compatible with the embedded module.
@@ -53,13 +94,15 @@ pub fn engine() -> wasmtime::Result<wasmtime::Engine> {
 /// `engine` must come from [`engine`] or use [`engine_config`] unchanged
 /// (plus runtime-only settings).
 ///
-/// The first load in a process maps the embedded bytes in place (no copy).
+/// The first load in a process maps the aligned bytes in place. On hosts whose
+/// linker/loader cannot preserve 64 KiB alignment, these bytes are copied once
+/// into cached aligned storage first.
 /// Wasmtime registers code by address, so the same bytes cannot back two live
 /// modules; every later load copies them instead (tens of milliseconds).
 /// Hosts should load once and share the module.
 pub fn load_module(engine: &wasmtime::Engine) -> wasmtime::Result<wasmtime::Module> {
     static MAPPED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
-    let bytes: &'static [u8] = &CWASM.0;
+    let bytes = cwasm_bytes();
     // Native code must live in executable memory, never the read-only static,
     // so it is always copied (the first load costs ~35 ms more than Pulley's).
     if cfg!(feature = "native") || MAPPED.swap(true, std::sync::atomic::Ordering::AcqRel) {
@@ -68,7 +111,7 @@ pub fn load_module(engine: &wasmtime::Engine) -> wasmtime::Result<wasmtime::Modu
     }
     // SAFETY: the bytes were produced by `Engine::precompile_module` in this
     // crate's build script with the same wasmtime version and `engine_config`,
-    // live in immutable static memory for the life of the process, and are
+    // live in immutable process-lifetime storage, and are
     // mapped by at most one module (guarded by `MAPPED`).
     unsafe { wasmtime::Module::deserialize_raw(engine, std::ptr::NonNull::from(bytes)) }
 }
@@ -97,6 +140,16 @@ mod tests {
         assert_eq!(cwasm_bytes().as_ptr() as usize % 65536, 0);
     }
 
+    #[test]
+    fn padded_module_preserves_bytes_and_alignment() {
+        for len in [0, 1, 127, 65535, 65536, 65537] {
+            let input: Vec<_> = (0..len).map(|i| (i % 251) as u8).collect();
+            let module = PaddedModule::new(&input);
+            assert_eq!(module.bytes().as_ptr() as usize % 65536, 0);
+            assert_eq!(module.bytes(), input);
+        }
+    }
+
     #[test]
     fn stdlib_zip_is_a_zip() {
         assert_eq!(&STDLIB_ZIP[..4], b"PK\x03\x04");

crates/bashkit/docs/cpython.md

@@ -44,7 +44,9 @@ the guest never runs native code in your process.
 The interpreter loads on the first call of a process: the first `python3`
 takes about 19 ms on the reference machine, later calls about 4.4 ms. To move
 that one-time cost out of the first request, call
-`bashkit::CPython::warm_up()` at startup.
+`bashkit::CPython::warm_up()` at startup. On hosts where the linker or loader
+cannot preserve 64 KiB module alignment, such as macOS, an aligned immutable
+copy is cached once for the process; later calls reuse it.
 
 ## Why CPython instead of Monty
 

crates/bashkit/docs/threat-model.md

@@ -111,7 +111,7 @@ through configurable limits.
 | Brace range OOM (TM-DOS-041) | `{1..999999999}` | Cap range size | **MITIGATED** |
 | Brace combinatorial (TM-DOS-042) | `{1..100}{1..100}{1..100}` | Cap total expansion | **MITIGATED** |
 | Compound assign overflow (TM-DOS-043) | `((x+=1))` with x=i64::MAX | `wrapping_*` ops | **MITIGATED** |
-| Lexer stack overflow (TM-DOS-044) | ~50 nested `$()` in quotes | Depth tracking | **MITIGATED** |
+| Lexer stack overflow (TM-DOS-044) | ~50 nested `$()` in quotes | Shared AST depth, heap-owned child parsers, outlined nonrecursive parser frames | **MITIGATED** |
 | parse_word_string limits (TM-DOS-050) | Parameter expansion ignores limits | Propagate limits | **MITIGATED** |
 | Removed YAML helper parser (TM-DOS-051) | Former custom parser recursed over indentation | Parser deleted; yq uses TM-DOS-101 controls | **REMOVED** |
 | Template engine recursion (TM-DOS-052) | Nested `{{#if}}`/`{{#each}}` overflow | Add depth limit | **MITIGATED** |
@@ -192,7 +192,7 @@ let bash = Bash::builder()
 | jq control normalization amplification (TM-DOS-100) | Literal controls expand sixfold as `\u00XX` | Charge single-pass work and lease live bytes before allocation growth | MITIGATED |
 | yq structured-data amplification (TM-DOS-101) | Deep/multi-document YAML or JSON, runaway filters, expanded output | Parser depth and 4096-document caps, aggregate budgets, shared jaq work/deadline/output limits, final render cap | MITIGATED |
 | Archive decoder pre-allocation (TM-DOS-102) | Compressed output grows before memory checks | Validate and charge each decoder chunk before reserve/copy | MITIGATED |
-| Post-allocation charging (TM-DOS-103) | Archive/compression buffers grow before their live-byte check | Owning budget-aware string/vector/byte builders charge before reserve, roll back errors, and release on drop | MITIGATED |
+| Post-allocation charging (TM-DOS-103) | Archive/compression buffers or tar file-list names grow before their live-byte check | Owning budget-aware string/vector/byte builders charge before reserve, roll back errors, and release on drop; tar file lists also lease raw reads and charge per-line work | MITIGATED |
 | Pipeline stderr aggregation (TM-DOS-104) | Many stderr-producing pipeline stages accumulate past the output cap | Stop appending at `max_stderr_bytes` and propagate the truncation flag to the result | MITIGATED |
 | Zip creation pre-allocation (TM-DOS-106) | Recursive inputs and encoded output allocate before live-memory checks | Collect paths only, lease file sizes before VFS reads, and encode through a budget-aware byte buffer | MITIGATED |
 | Analysis command-name validation (TM-DOS-107) | A huge comment before thousands of commands makes validation rescan the source per command | Index source character positions once, then binary-search each name | MITIGATED |
@@ -656,15 +656,18 @@ The parser includes multiple layers of depth protection to prevent stack overflo
 attacks:
 
 1. **Configurable depth limit** (`max_ast_depth`, default 100): Controls maximum nesting
-   of compound commands (if/for/while/case/subshell).
+   of compound commands (if/for/while/case/subshell) and substitutions.
 
 2. **Hard cap** (`HARD_MAX_AST_DEPTH = 100`): Even if the caller configures a higher
    `max_ast_depth`, the parser clamps it to 100. This prevents misconfiguration from
    causing stack overflow.
 
 3. **Child parser inheritance** (TM-DOS-021): When parsing `$(...)` or `<(...)`,
-   the child parser inherits the *remaining* depth budget and fuel from the parent.
-   This prevents attackers from bypassing depth limits through nested substitutions.
+   each substitution consumes one level of the enclosing AST depth. Command
+   substitution child parsers inherit the same ceiling, remaining fuel, execution
+   budget, and timeout origin, and stay heap-owned while parsing. Nonrecursive
+   parameter parsing and AST assembly run outside recursive frames, keeping the
+   unchanged hard ceiling safe on default 2 MiB Rust test-thread stacks.
 
 4. **Arithmetic depth limit** (TM-DOS-026): The arithmetic evaluator (`$((expr))`)
    has its own depth limit (`MAX_ARITHMETIC_DEPTH = 50`) to prevent stack overflow

crates/bashkit/src/builtins/archive.rs

@@ -10,6 +10,9 @@
 //! normalize it to the regular short-option path instead of maintaining a
 //! second parser.
 //!
+//! File lists retain budget-owned names, never an eager `Vec<String>`; raw
+//! reads are admitted by metadata size and every line checks shared work.
+//!
 //! bzip2 0.6 uses the maintained pure-Rust libbz2-rs backend by default. Its
 //! fixed codec state plus bounded, pre-charged output avoids native-library
 //! deployment differences while retaining standard bzip2 CRC validation.
@@ -77,6 +80,29 @@ fn budgeted_vec<T>(ctx: &Context<'_>) -> Result<BudgetedVec<T>> {
     with_execution_budget(ctx, |budget| BudgetedVec::new(budget).map_err(Into::into))
 }
 
+// THREAT[TM-DOS-103]: Decode directly into admitted storage, including UTF-8
+// replacement expansion. `from_utf8_lossy(...).into_owned()` allocates first.
+fn tar_list_name(ctx: &Context<'_>, mut bytes: &[u8]) -> Result<BudgetedString> {
+    let mut name = budgeted_string(ctx)?;
+    while !bytes.is_empty() {
+        ctx.consume_budget_work(1)?;
+        match std::str::from_utf8(bytes) {
+            Ok(text) => {
+                name.try_push_str(text)?;
+                break;
+            }
+            Err(error) => {
+                let valid = error.valid_up_to();
+                name.try_push_str(std::str::from_utf8(&bytes[..valid]).unwrap())?;
+                name.try_push('\u{fffd}')?;
+                let invalid = error.error_len().unwrap_or(bytes.len() - valid);
+                bytes = &bytes[valid + invalid..];
+            }
+        }
+    }
+    Ok(name)
+}
+
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
 enum ArchiveCompression {
     #[default]
@@ -397,34 +423,66 @@ impl Builtin for Tar {
 
         // `-T FILE` / `--files-from=FILE`: one member name per line, blank
         // lines skipped, `-` reads stdin.
-        let mut listed_names: Vec<String> = Vec::new();
+        // THREAT[TM-DOS-103]: Both name capacity and vector slots must be
+        // charged before allocation, even when the members do not exist.
+        let mut listed_names = budgeted_vec(&ctx)?;
         for list in &files_from {
+            let missing_list = || {
+                ExecResult::err(
+                    format!(
+                        "tar: {list}: Cannot stat: No such file or directory\ntar: Error is not recoverable: exiting now\n"
+                    ),
+                    2,
+                )
+            };
+            let bytes;
+            let _read_lease;
+            let _extra_lease;
             let content = if list == "-" {
-                String::from_utf8_lossy(ctx.stdin_bytes().unwrap_or_default()).into_owned()
+                let input = ctx.stdin_bytes().unwrap_or_default();
+                ctx.consume_budget_input(input.len())?;
+                input
             } else {
                 let path = resolve_path(ctx.cwd, list);
-                match ctx.fs.read_file(&path).await {
-                    Ok(bytes) => String::from_utf8_lossy(&bytes).into_owned(),
-                    Err(_) => {
-                        return Ok(ExecResult::err(
-                            format!(
-                                "tar: {list}: Cannot stat: No such file or directory\ntar: Error is not recoverable: exiting now\n"
-                            ),
-                            2,
-                        ));
+                let metadata = match ctx.fs.stat(&path).await {
+                    Ok(metadata) => metadata,
+                    Err(error @ (crate::Error::ResourceLimit(_) | crate::Error::Cancelled)) => {
+                        return Err(error);
                     }
-                }
+                    Err(_) => return Ok(missing_list()),
+                };
+                let size = usize::try_from(metadata.size).unwrap_or(usize::MAX);
+                ctx.consume_budget_input(size)?;
+                _read_lease = ctx.lease_budget_bytes(size)?;
+                bytes = match ctx.fs.read_file(&path).await {
+                    Ok(bytes) => bytes,
+                    Err(error @ (crate::Error::ResourceLimit(_) | crate::Error::Cancelled)) => {
+                        return Err(error);
+                    }
+                    Err(_) => return Ok(missing_list()),
+                };
+                // Host-backed files may grow between stat/read; the backend
+                // owns read-allocation admission, as for other VFS consumers.
+                ctx.consume_budget_input(bytes.len().saturating_sub(size))?;
+                _extra_lease = ctx.lease_budget_bytes(bytes.capacity().saturating_sub(size))?;
+                bytes.as_slice()
             };
-            ctx.consume_budget_input(content.len())?;
-            listed_names.extend(
-                content
-                    .lines()
-                    .filter(|l| !l.is_empty())
-                    .map(str::to_string),
-            );
+            ctx.consume_budget_work(u64::try_from(content.len().div_ceil(64)).unwrap_or(u64::MAX))?;
+            for line in content.split_inclusive(|byte| *byte == b'\n') {
+                ctx.consume_budget_work(1)?;
+                // Match str::lines(): strip CR only as part of CRLF.
+                let line = if let Some(line) = line.strip_suffix(b"\n") {
+                    line.strip_suffix(b"\r").unwrap_or(line)
+                } else {
+                    line
+                };
+                if !line.is_empty() {
+                    listed_names.try_push(tar_list_name(&ctx, line)?)?;
+                }
+            }
         }
-        for name in &listed_names {
-            files.try_push(name.as_str())?;
+        for name in &*listed_names {
+            files.try_push(&**name)?;
         }
 
         // Check for exactly one of -c, -x, -t
@@ -1894,6 +1952,36 @@ mod tests {
 
     // ==================== tar tests ====================
 
+    #[tokio::test]
+    async fn tar_files_from_lossy_decoder_matches_std() {
+        let (fs, mut cwd, mut variables) = create_test_ctx().await;
+        let env = HashMap::new();
+        let ctx = Context {
+            args: &[],
+            env: &env,
+            variables: &mut variables,
+            cwd: &mut cwd,
+            fs,
+            stdin: None,
+            #[cfg(feature = "http_client")]
+            http_client: None,
+            #[cfg(feature = "git")]
+            git_client: None,
+            #[cfg(feature = "ssh")]
+            ssh_client: None,
+            shell: None,
+        };
+        // Valid prefixes, malformed multi-byte sequences, and incomplete
+        // tails must retain the previous standard-library decoding semantics.
+        for byte in 0..=255 {
+            let bytes = [b'A', byte, 0xf0, 0x9f, 0x92, byte, b'Z', 0xe2, 0x82];
+            for input in [bytes.as_slice(), &bytes[..7]] {
+                let name = tar_list_name(&ctx, input).unwrap();
+                assert_eq!(&*name, String::from_utf8_lossy(input).as_ref());
+            }
+        }
+    }
+
     #[tokio::test]
     async fn test_tar_create_and_list() {
         let (fs, mut cwd, mut variables) = create_test_ctx().await;