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.
This commit was merged in pull request #4.
This commit is contained in:
+82
-3
@@ -42,15 +42,53 @@ fn main() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// 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 {
|
||||
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))
|
||||
.place_data_file(format!("registry/{image}/initrd"))
|
||||
.context("Failed to write to XDG_DATA_HOME")?;
|
||||
Ok(initrd_path
|
||||
let reg_dir = initrd_path
|
||||
.parent()
|
||||
.expect("data file has a parent")
|
||||
.to_owned())
|
||||
.to_owned();
|
||||
|
||||
// Defense in depth: the constructed path must stay inside the registry.
|
||||
let registry_root = base
|
||||
.place_data_file("registry/.root-check")
|
||||
.context("Failed to write to XDG_DATA_HOME")?
|
||||
.parent()
|
||||
.expect("data file has a parent")
|
||||
.to_owned();
|
||||
if !reg_dir.starts_with(®istry_root) {
|
||||
bail!(
|
||||
"invalid image name '{image}': registry path escapes {}",
|
||||
registry_root.display()
|
||||
);
|
||||
}
|
||||
Ok(reg_dir)
|
||||
}
|
||||
|
||||
fn build(image: &str) -> Result<()> {
|
||||
@@ -207,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"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user