Compare commits

...
1 Commits
Author SHA1 Message Date
marvin cb87541151 fix: validate image name before building registry path
CI / build (pull_request) Successful in 11s
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.
2026-08-26 20:10:32 +00:00
+89 -5
View File
@@ -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<PathBuf> {
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(&registry_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(&reg_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"
);
}
}
}