diff --git a/src/persistent_tmp.rs b/src/persistent_tmp.rs index 9968923..13eb62d 100644 --- a/src/persistent_tmp.rs +++ b/src/persistent_tmp.rs @@ -3,15 +3,14 @@ use std::io; use std::os::fd::{AsRawFd, OwnedFd}; use std::os::unix::fs::{DirBuilderExt, MetadataExt, OpenOptionsExt}; use std::path::PathBuf; +use std::process::Command; use crate::SandboxConfig; use crate::errors::SandboxError; pub struct PersistentTmpDirs { - pub tmp: PathBuf, - pub var_tmp: PathBuf, - // Held to keep `/proc/self/fd/` bind sources valid through `exec()`. - _keep_fds: Vec, + tmp: BindSource, + var_tmp: BindSource, } impl PersistentTmpDirs { @@ -25,20 +24,40 @@ impl PersistentTmpDirs { if config.dry_run { return Ok(Some(Self { - tmp: tmp.host_path, - var_tmp: var_tmp.host_path, - _keep_fds: Vec::new(), + tmp: BindSource::HostPath(tmp.host_path), + var_tmp: BindSource::HostPath(var_tmp.host_path), })); } - let tmp_fd = tmp.open()?; - let var_tmp_fd = var_tmp.open()?; Ok(Some(Self { - tmp: format!("/proc/self/fd/{}", tmp_fd.as_raw_fd()).into(), - var_tmp: format!("/proc/self/fd/{}", var_tmp_fd.as_raw_fd()).into(), - _keep_fds: vec![tmp_fd, var_tmp_fd], + tmp: BindSource::OpenDir(tmp.open()?), + var_tmp: BindSource::OpenDir(var_tmp.open()?), })) } + + pub fn add_binds(&self, cmd: &mut Command) { + self.tmp.add_bind(cmd, "/tmp"); + self.var_tmp.add_bind(cmd, "/var/tmp"); + } +} + +enum BindSource { + HostPath(PathBuf), + /// bwrap closes a `--bind-fd` descriptor once mounted, so the sandboxed + /// command never inherits this handle on the host filesystem. + OpenDir(OwnedFd), +} + +impl BindSource { + fn add_bind(&self, cmd: &mut Command, target: &str) { + match self { + Self::HostPath(path) => cmd.arg("--bind").arg(path).arg(target), + Self::OpenDir(fd) => cmd + .arg("--bind-fd") + .arg(fd.as_raw_fd().to_string()) + .arg(target), + }; + } } pub struct PersistentTmpDir { diff --git a/src/sandbox.rs b/src/sandbox.rs index 5126e14..dc03549 100644 --- a/src/sandbox.rs +++ b/src/sandbox.rs @@ -250,10 +250,7 @@ fn add_whitelist_mode( cmd.arg("--tmpfs").arg(&cache_dir); match persistent { - Some(p) => { - cmd.arg("--bind").arg(&p.tmp).arg("/tmp"); - cmd.arg("--bind").arg(&p.var_tmp).arg("/var/tmp"); - } + Some(p) => p.add_binds(cmd), None => { cmd.args(["--tmpfs", "/tmp"]); cmd.args(["--tmpfs", "/var/tmp"]); diff --git a/tests/e2e/mounts.rs b/tests/e2e/mounts.rs index f70aee8..c8f4d10 100644 --- a/tests/e2e/mounts.rs +++ b/tests/e2e/mounts.rs @@ -463,6 +463,39 @@ fn persistent_tmp_survives_across_invocations() { ); } +#[test] +fn persistent_tmp_leaks_no_descriptor_into_host_tmp() { + let label = format!("e2e-fd-escape-{}-{}", std::process::id(), rand_suffix()); + let _cleanup_tmp = CleanupPath::new(format!("/tmp/agent-sandbox-{label}")); + let _cleanup_var = CleanupPath::new(format!("/var/tmp/agent-sandbox-{label}")); + let canary_name = format!("agent-sandbox-fd-canary-{label}"); + let canary_host = format!("/tmp/{canary_name}"); + let _cleanup_canary = CleanupPath::new(&canary_host); + fs::write(&canary_host, "host-secret").unwrap(); + + let output = Sandbox::new(&["--persistent-tmp", "--persistent-key", &label]) + .args([ + "--", + "bash", + "-c", + &format!( + "for fd in /proc/self/fd/*; do cat \"$fd/../{canary_name}\" 2>/dev/null; done; true" + ), + ]) + .output() + .expect("agent-sandbox binary failed to execute"); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !String::from_utf8_lossy(&output.stdout).contains("host-secret"), + "an inherited descriptor reaches host /tmp" + ); +} + #[test] fn persistent_tmp_is_noop_in_blacklist_mode() { let label = format!("e2e-bl-noop-{}-{}", std::process::id(), rand_suffix());