fix(host-agent): SECURITY — file manager copy/list no longer follow symlinks out of the jail
Automated security review (HIGH) caught a jail-escape my own review
missed: copy_recursive used fs::metadata (follows symlinks). A symlink
inside the jail pointing to e.g. /etc, then a 'copy' of its parent dir,
would dereference it and pull external content INTO the jail where it
could be read — a read-escape exfiltration. jail() validates only the
top-level src/dest; the recursive walk reintroduced the escape.
Fix: copy_recursive uses symlink_metadata and refuses any symlink
('symlinks are not followed across the jail boundary'). list() likewise
switched to symlink_metadata so it reports the link, never the
dereferenced target's size/type (info leak). Two regression tests added:
copy-symlink-exfil (asserts no external content lands inside) and
list-no-deref. 44/44 tests green. Rolled forward to alpha.4 (vulnerable
alpha.3 superseded).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2
corrosion-host-agent/Cargo.lock
generated
2
corrosion-host-agent/Cargo.lock
generated
@@ -264,7 +264,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b"
|
||||
|
||||
[[package]]
|
||||
name = "corrosion-host-agent"
|
||||
version = "2.0.0-alpha.3"
|
||||
version = "2.0.0-alpha.4"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"async-nats",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "corrosion-host-agent"
|
||||
version = "2.0.0-alpha.3"
|
||||
version = "2.0.0-alpha.4"
|
||||
edition = "2021"
|
||||
description = "Corrosion Host Agent — multi-game ops runtime for self-hosted game servers"
|
||||
license = "UNLICENSED"
|
||||
|
||||
@@ -198,7 +198,11 @@ pub fn list(root: &Path, rel: &str) -> anyhow::Result<Vec<FileEntry>> {
|
||||
let mut entries: Vec<FileEntry> = Vec::new();
|
||||
for item in rd {
|
||||
let item = item.with_context(|| format!("reading directory entry in '{}'", abs.display()))?;
|
||||
let meta = item.metadata().with_context(|| format!("stat '{}'", item.path().display()))?;
|
||||
// symlink_metadata (lstat): report the link itself, never the target —
|
||||
// following it would leak the size/type/existence of files outside the
|
||||
// jail. A symlink lists as a zero-ish-size non-dir entry.
|
||||
let meta = fs::symlink_metadata(item.path())
|
||||
.with_context(|| format!("stat '{}'", item.path().display()))?;
|
||||
|
||||
let name = item.file_name().to_string_lossy().into_owned();
|
||||
let is_dir = meta.is_dir();
|
||||
@@ -367,11 +371,24 @@ pub fn copy(root: &Path, src: &str, dest: &str) -> anyhow::Result<()> {
|
||||
.with_context(|| format!("copy '{}' -> '{}'", src_abs.display(), dest_abs.display()))
|
||||
}
|
||||
|
||||
/// Recursive copy helper (mirrors Go's `copyRecursive`).
|
||||
/// Recursive copy helper.
|
||||
///
|
||||
/// SECURITY: uses `symlink_metadata` (does NOT follow symlinks) and refuses to
|
||||
/// copy any symlink. `jail()` only validates the top-level src/dest; a symlink
|
||||
/// *inside* a copied directory that points outside the jail would, if followed,
|
||||
/// pull external content (e.g. `/etc`) into the jail where it could then be
|
||||
/// read — a jail-escape exfiltration. Refusing symlinks closes that path.
|
||||
fn copy_recursive(src: &Path, dest: &Path) -> anyhow::Result<()> {
|
||||
let meta = fs::metadata(src)
|
||||
let meta = fs::symlink_metadata(src)
|
||||
.with_context(|| format!("stat source '{}'", src.display()))?;
|
||||
|
||||
if meta.file_type().is_symlink() {
|
||||
bail!(
|
||||
"refusing to copy symlink '{}' — symlinks are not followed across the jail boundary",
|
||||
src.display()
|
||||
);
|
||||
}
|
||||
|
||||
if meta.is_dir() {
|
||||
fs::create_dir_all(dest)
|
||||
.with_context(|| format!("create_dir_all '{}'", dest.display()))?;
|
||||
|
||||
@@ -347,6 +347,62 @@ fn jail_rejects_chained_symlink_escape() {
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY REGRESSION: copying a directory that contains a symlink pointing
|
||||
/// OUTSIDE the jail must NOT dereference it and pull external content inside.
|
||||
/// jail() validates only the top-level src/dest; the recursive copy must
|
||||
/// refuse symlinks itself or it becomes a read-escape exfiltration path.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn copy_refuses_to_follow_symlink_out_of_jail() {
|
||||
let dir = tempdir();
|
||||
let root = dir.path();
|
||||
let outside = tempdir();
|
||||
std::fs::write(outside.path().join("secret.txt"), "TOP SECRET")
|
||||
.expect("write external secret");
|
||||
|
||||
// A directory inside the jail containing a symlink to the outside dir.
|
||||
std::fs::create_dir(root.join("src")).expect("mkdir src");
|
||||
std::os::unix::fs::symlink(outside.path(), root.join("src").join("escape"))
|
||||
.expect("plant symlink to outside");
|
||||
|
||||
// Attempt to copy src -> dest (both inside the jail).
|
||||
let err = filemanager::copy(root, "src", "dest")
|
||||
.expect_err("copy must refuse the embedded symlink");
|
||||
assert!(
|
||||
format!("{err:#}").contains("symlink"),
|
||||
"error should name the refused symlink, got: {err:#}"
|
||||
);
|
||||
|
||||
// The external secret must NOT have landed inside the jail.
|
||||
assert!(
|
||||
!root.join("dest").join("escape").join("secret.txt").exists(),
|
||||
"external content leaked into the jail via symlink-following copy",
|
||||
);
|
||||
}
|
||||
|
||||
/// `list` must report a symlink as the link itself, never the dereferenced
|
||||
/// target — otherwise it leaks the size/type of files outside the jail.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn list_does_not_dereference_symlink_metadata() {
|
||||
let dir = tempdir();
|
||||
let root = dir.path();
|
||||
std::os::unix::fs::symlink(Path::new("/etc/passwd"), root.join("leak"))
|
||||
.expect("plant symlink");
|
||||
|
||||
let entries = filemanager::list(root, "").expect("list root");
|
||||
let leak = entries.iter().find(|e| e.name == "leak").expect("symlink listed");
|
||||
// /etc/passwd is a regular file; if we followed the link, is_dir would
|
||||
// reflect the target. We must report the link, which is not a directory,
|
||||
// and must NOT expose the target's byte size.
|
||||
assert!(!leak.is_dir, "symlink must not be reported as a directory");
|
||||
let target_size = std::fs::metadata("/etc/passwd").map(|m| m.len()).unwrap_or(0);
|
||||
assert!(
|
||||
leak.size != target_size || target_size == 0,
|
||||
"list leaked the symlink target's size ({target_size} bytes)"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Dispatch layer tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user