From cb87541151a07a6ff47cf1e6b8acdcdd514b457f Mon Sep 17 00:00:00 2001 From: Marvin Date: Wed, 26 Aug 2026 20:10:32 +0000 Subject: [PATCH] fix: validate image name before building registry path The user-supplied image/name argument was interpolated unsanitized into registry/{name}/initrd, so a name containing '../' (e.g. 'slim run ../../.ssh/authorized_keys') escaped the XDG data dir and let slim create directories and read/write files at attacker-chosen locations. validate_image_name now rejects empty names, '.', '..' and anything outside [A-Za-z0-9._:-] at the registry_dir chokepoint used by both build and run, plus a defense-in-depth containment check that the resolved path stays under the registry root. Fixes sec-2 from the code review. --- src/main.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index aca50d5..8823730 100644 --- a/src/main.rs +++ b/src/main.rs @@ -42,10 +42,53 @@ fn main() -> Result<()> { Ok(()) } -fn registry_dir(image: &str) -> PathBuf { +/// Validate a user-supplied image name before it is used in a registry path. +/// +/// The name becomes a directory under `$XDG_DATA_HOME/slim-rs/registry/`, so +/// path separators and `.`/`..` would allow escaping that directory. +/// Allow-list: ASCII alphanumerics plus `.`, `_`, `:`, `-` (covers image +/// refs like `alpine:3.15`). +fn validate_image_name(name: &str) -> Result<()> { + let valid = !name.is_empty() + && name != "." + && name != ".." + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | ':' | '-')); + if !valid { + anyhow::bail!( + "invalid image name '{name}': must be non-empty and contain only \ + alphanumerics, '.', '_', ':' and '-' (no path separators or '..')" + ); + } + Ok(()) +} + +fn registry_dir(image: &str) -> Result { + validate_image_name(image)?; let base = xdg::BaseDirectories::with_prefix("slim-rs"); - let initrd_path = base.place_data_file(format!("registry/{}/initrd", image)); - initrd_path.unwrap().parent().unwrap().to_path_buf() + let initrd_path = base + .place_data_file(format!("registry/{image}/initrd")) + .context("Failed to resolve XDG data home")?; + let reg_dir = initrd_path + .parent() + .context("registry path has no parent")? + .to_path_buf(); + + // Defense in depth: the constructed path must stay inside the registry. + let registry_root = base + .place_data_file("registry/.root-check") + .context("Failed to resolve XDG data home")? + .parent() + .context("registry path has no parent")? + .to_path_buf(); + if !reg_dir.starts_with(®istry_root) { + anyhow::bail!( + "invalid image name '{image}': registry path escapes {}", + registry_root.display() + ); + } + Ok(reg_dir) } fn build(image: &str) -> Result<()> { @@ -100,7 +143,7 @@ fn build_artifacts(image: &str, mount_path: &Path) -> Result<()> { ); } - let reg_dir = registry_dir(image); + let reg_dir = registry_dir(image)?; fs::create_dir_all(®_dir)?; println!("Registry: {}", reg_dir.display()); @@ -156,7 +199,7 @@ fn build_artifacts(image: &str, mount_path: &Path) -> Result<()> { } fn run(name: &str) -> Result<()> { - let reg_dir = registry_dir(name); + let reg_dir = registry_dir(name)?; let vmlinuz_path = reg_dir.join("vmlinuz"); let initrd_path = reg_dir.join("initrd"); if !vmlinuz_path.exists() || !initrd_path.exists() { @@ -202,3 +245,44 @@ fn run(name: &str) -> Result<()> { } Ok(()) } + +#[cfg(test)] +mod tests { + use super::validate_image_name; + + #[test] + fn accepts_valid_image_names() { + for name in [ + "alpine:3.15", + "slim", + "mock-vm", + "my_vm", + "a.b.c", + "vm-1.2.3", + ] { + validate_image_name(name).unwrap_or_else(|e| panic!("{name} rejected: {e}")); + } + } + + #[test] + fn rejects_traversal_and_separators() { + for name in [ + "", + ".", + "..", + "../evil", + "../../tmp/evil", + "a/b", + "/abs", + "a\\b", + "foo bar", + "vm;rm", + "vm\n", + ] { + assert!( + validate_image_name(name).is_err(), + "{name:?} should be rejected" + ); + } + } +}