Skip to content

Commit c9db71d

Browse files
committed
fix(oci): confine untrusted layer titles to the output directory
1 parent 85d5b8e commit c9db71d

2 files changed

Lines changed: 62 additions & 2 deletions

File tree

crates/soar-dl/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ pub enum DownloadError {
4141
#[diagnostic(code(soar_dl::layer_not_found))]
4242
LayerNotFound,
4343

44+
#[error("Unsafe layer path in manifest: {title}")]
45+
#[diagnostic(code(soar_dl::unsafe_layer_path))]
46+
UnsafeLayerPath { title: String },
47+
4448
#[error("Invalid response from server")]
4549
#[diagnostic(code(soar_dl::invalid_response))]
4650
InvalidResponse,

crates/soar-dl/src/oci.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,19 @@ impl OciLayer {
148148
}
149149
}
150150

151+
/// Resolves an untrusted OCI layer title into a path confined to `output_dir`.
152+
///
153+
/// Only the final path component of the title is used, so titles such as
154+
/// `../../etc/passwd`, `/etc/passwd`, or `..` cannot escape `output_dir`.
155+
fn safe_layer_path(output_dir: &Path, title: &str) -> Result<PathBuf, DownloadError> {
156+
let name = Path::new(title)
157+
.file_name()
158+
.ok_or_else(|| DownloadError::UnsafeLayerPath {
159+
title: title.to_string(),
160+
})?;
161+
Ok(output_dir.join(name))
162+
}
163+
151164
#[derive(Clone)]
152165
pub struct OciDownload {
153166
reference: OciReference,
@@ -465,7 +478,7 @@ impl OciDownload {
465478

466479
for layer in layers {
467480
let filename = layer.title().unwrap();
468-
let path = output_dir.join(filename);
481+
let path = safe_layer_path(output_dir, filename)?;
469482

470483
if path.is_file() {
471484
if let Ok(metadata) = path.metadata() {
@@ -566,7 +579,13 @@ impl OciDownload {
566579
Some(f) => f,
567580
None => continue,
568581
};
569-
let path = output_dir.join(filename);
582+
let path = match safe_layer_path(&output_dir, filename) {
583+
Ok(p) => p,
584+
Err(e) => {
585+
errors.lock().unwrap().push(format!("{e}"));
586+
continue;
587+
}
588+
};
570589

571590
if path.is_file() {
572591
if let Ok(metadata) = path.metadata() {
@@ -956,6 +975,43 @@ fn download_layer_impl(
956975
mod tests {
957976
use super::*;
958977

978+
#[test]
979+
fn safe_layer_path_keeps_plain_name_inside_output_dir() {
980+
let out = Path::new("/tmp/soar-out");
981+
let path = safe_layer_path(out, "tool.AppImage").unwrap();
982+
assert_eq!(path, out.join("tool.AppImage"));
983+
}
984+
985+
#[test]
986+
fn safe_layer_path_strips_traversal_and_absolute_titles() {
987+
let out = Path::new("/tmp/soar-out");
988+
// `..`, nested escapes, and absolute paths all collapse to the final
989+
// component joined under the output dir.
990+
assert_eq!(
991+
safe_layer_path(out, "../../etc/passwd").unwrap(),
992+
out.join("passwd")
993+
);
994+
assert_eq!(
995+
safe_layer_path(out, "/etc/cron.d/evil").unwrap(),
996+
out.join("evil")
997+
);
998+
assert_eq!(safe_layer_path(out, "a/b/c").unwrap(), out.join("c"));
999+
}
1000+
1001+
#[test]
1002+
fn safe_layer_path_rejects_titles_without_a_final_component() {
1003+
let out = Path::new("/tmp/soar-out");
1004+
for bad in ["", "..", "foo/..", "/", "."] {
1005+
assert!(
1006+
matches!(
1007+
safe_layer_path(out, bad),
1008+
Err(DownloadError::UnsafeLayerPath { .. })
1009+
),
1010+
"expected {bad:?} to be rejected"
1011+
);
1012+
}
1013+
}
1014+
9591015
#[test]
9601016
fn test_oci_reference_from_str_simple() {
9611017
let reference = OciReference::from("org/repo:tag");

0 commit comments

Comments
 (0)